Skip to content

Commit 105c1df

Browse files
laitingshenggithub-actions[bot]prekshivyasapurvvkumariarsliter
authored
fix(uninstall): do not fail cleanup on another user's gateway process (#9005)
## Summary `nemoclaw uninstall` exited 1 on a multi-user host whenever another user's `openshell-gateway` process was running, because the host gateway scan signalled every process that `pgrep` returned and counted the resulting permission error as a cleanup failure. Uninstall now keeps a gateway process that another non-root user owns running, names it, and continues with the remaining cleanup. A `root`-owned process and the gateway process that this installation recorded still block cleanup as before. ## Related Issue Fixes #8996 ## Changes - `src/lib/onboard/host-gateway-process.ts`: classify a swept process by owner. A process that another non-root user owns, and that this installation did not record in its PID file, is reported through a new optional `foreignUserPids` result field and left running instead of counting as a failure. Gateway-scoped stops and `root`-owned processes are unchanged. - `src/lib/onboard/host-gateway-process.test.ts`: cover foreign-user preservation, current-user and root-owned handling, and the PID-file exception that still stops a process recorded by this installation. - `src/lib/actions/uninstall/run-plan-foreign-user-gateway.test.ts`: cover the uninstall exit status for another user's process and for a current-user process that cannot be stopped. - `docs/manage-sandboxes/uninstall-nemoclaw.mdx`, `docs/reference/commands.mdx`, `docs/reference/troubleshooting.mdx`: explain which processes uninstall stops, when preservation can still exit successfully, and why another user's process can keep a port in use. - `agents/hermes/Dockerfile`, `agents/hermes/validate-env-secret-boundary.py`, `test/hermes-env-secret-boundary-hardening.test.ts`: admit and validate the provider-assigned Hermes API port required by the current managed-image runtime. `foreignUserPids` is optional because `src/lib/actions/sandbox/destroy-gateway.ts`, `src/lib/onboard/docker-driver-gateway-prelaunch.ts`, and `src/lib/tunnel/gateway-port-release-test-helpers.ts` build this result type directly, and a required field would change all three without a behavior need. `src/lib/actions/uninstall/run-plan-foreign-user-gateway.test.ts` protects the uninstall exit-code contract that depends on it. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: The current changes preserve process-ownership boundaries and keep the Hermes runtime environment allowlist constrained to the validated numeric API-port field. Raw-secret rejection and supervisor-identity checks remain covered. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Updated `docs/manage-sandboxes/uninstall-nemoclaw.mdx`, `docs/reference/commands.mdx`, and `docs/reference/troubleshooting.mdx` to document foreign-user gateway preservation, exit-status behavior, recorded and root-owned process handling, and port remediation. The signed main merge preserved these #9005 changes, and the documentation matches the related uninstall behavior tests and follows `WRITING.md` and `docs/CONTRIBUTING.md`. - Agent: Codex documentation writer <!-- docs-review-head-sha: 1ac20a1 --> <!-- docs-review-agents-blob-sha: e30afb2 --> ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run --project cli src/lib/onboard/host-gateway-process.test.ts src/lib/actions/uninstall/run-plan-foreign-user-gateway.test.ts` — 2 files, 21 tests passed; `npx vitest run test/hermes-env-secret-boundary-hardening.test.ts test/hermes-api-port-startup.test.ts test/hermes-openshell-runtime-env-boundary.test.ts` — 3 files, 33 tests passed, 1 platform-specific skip; the OpenShell 0.0.101 managed-bootstrap activation passed locally - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] 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) --- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Uninstall now avoids stopping unrecorded gateway processes owned by another non-root user. * Uninstall reports the process owner and PID when a gateway remains active. * Stop failures provide `sudo kill -9` guidance. * Scoped and all-port uninstall operations now return a failure status when processes cannot be stopped, while full uninstall continues cleanup. * **Documentation** * Added guidance for resolving gateway port conflicts caused by another user's process. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
1 parent e25ae07 commit 105c1df

6 files changed

Lines changed: 248 additions & 1 deletion

File tree

docs/manage-sandboxes/uninstall-nemoclaw.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,13 @@ Rerun `NEMOCLAW_GATEWAY_PORT=<port> $$nemoclaw uninstall` with the gateway port
115115
For an externally supervised authority, uninstall preserves the local gateway state used by the running process in both full and gateway-scoped cleanup.
116116
It also preserves the gateway process, supervisor resources, marked Linux unit, Docker resources, OpenShell binaries, and the declared external state directory.
117117
A custom-port uninstall does not stop or remove the default gateway service or its environment file.
118+
Uninstall does not stop an `openshell-gateway` process that another non-root user owns and that this installation did not record.
119+
It names the owner and process ID, leaves that process running, and continues with the remaining cleanup.
120+
If no other cleanup fails, uninstall exits with status `0` even though that process can keep its port in use.
121+
Uninstall still tries to stop a `root`-owned process and the gateway process that this installation recorded.
122+
If either of those stops fails, uninstall prints `sudo kill -9 <pid>` for the process.
123+
A gateway-scoped uninstall and every `--all-gateway-ports` pass exit nonzero after that failure.
124+
A single full uninstall reports the process and continues.
118125
Before scoped cleanup stops a Docker gateway process, including a managed default gateway service, NemoClaw requires two Docker namespace proofs.
119126
The selected Docker gateway configuration and any running gateway process must use the state-root-specific OpenShell sandbox namespace that NemoClaw generated.
120127
Because the supported OpenShell Podman schema does not expose `sandbox_namespace`, scoped Podman uninstall fails closed before signaling and preserves the gateway runtime evidence and local state.

docs/reference/commands.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4153,6 +4153,13 @@ Rerun `NEMOCLAW_GATEWAY_PORT=<port> $$nemoclaw uninstall` with the gateway port
41534153
For an externally supervised authority, uninstall preserves the selected local gateway state in both full and gateway-scoped cleanup.
41544154
It also preserves the gateway process, supervisor resources, marked Linux unit, Docker resources, OpenShell binaries, and the declared external state directory.
41554155
A custom-port uninstall does not stop or remove the default gateway service or its environment file.
4156+
Uninstall does not stop an `openshell-gateway` process that another non-root user owns and that this installation did not record.
4157+
It names the owner and process ID, leaves that process running, and continues with the remaining cleanup.
4158+
If no other cleanup fails, uninstall exits with status `0` even though that process can keep its port in use.
4159+
Uninstall still tries to stop a `root`-owned process and the gateway process that this installation recorded.
4160+
If either of those stops fails, uninstall prints `sudo kill -9 <pid>` for the process.
4161+
A gateway-scoped uninstall and every `--all-gateway-ports` pass exit nonzero after that failure.
4162+
A single full uninstall reports the process and continues.
41564163
Before scoped cleanup stops a Docker gateway process, including a managed default gateway service, NemoClaw requires two Docker namespace proofs.
41574164
The selected Docker gateway configuration and any running gateway process must use the state-root-specific OpenShell sandbox namespace that NemoClaw generated.
41584165
Because the supported OpenShell Podman schema does not expose `sandbox_namespace`, scoped Podman uninstall fails closed before signaling and preserves the gateway runtime evidence and local state.

docs/reference/troubleshooting.mdx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,9 @@ openshell gateway list
432432

433433
If the gateway name and its port-scoped state remain, treat it as a second environment and select that port for cleanup.
434434
If the gateway is absent but the port still listens, cleanup did not stop the listener; follow the process or service remediation printed by uninstall before you retry.
435+
If uninstall reported that it kept an `openshell-gateway` process owned by another user running, that process still holds the port.
436+
This can happen after uninstall exits successfully because NemoClaw does not treat another user's process as a cleanup failure.
437+
Ask that user to stop the process, or onboard under a different `NEMOCLAW_GATEWAY_PORT`.
435438

436439
Remove one environment by selecting its port:
437440

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import fs from "node:fs";
5+
import os from "node:os";
6+
import path from "node:path";
7+
8+
import { describe, expect, it, vi } from "vitest";
9+
10+
import { type RunResult, runUninstallPlan } from "./run-plan";
11+
12+
const HOST_GATEWAY_PID = 9999043;
13+
14+
function ok(stdout = ""): RunResult {
15+
return { status: 0, stdout, stderr: "" };
16+
}
17+
18+
function notFound(): RunResult {
19+
return { status: 1, stdout: "", stderr: "" };
20+
}
21+
22+
function uninstallWithHostGatewayOwnedBy(uid: number): {
23+
errors: string[];
24+
exitCode: number;
25+
} {
26+
const tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-uninstall-foreign-"));
27+
const errors: string[] = [];
28+
const psResults = new Map<string, RunResult>([
29+
["stat=", ok("S\n")],
30+
["args=", ok("/usr/local/bin/openshell-gateway\n")],
31+
["user=", ok("otheruser\n")],
32+
["uid=", ok(`${uid}\n`)],
33+
]);
34+
const run = (command: string, args: string[]): RunResult =>
35+
command === "pgrep"
36+
? args.some((arg) => arg.includes("openshell-gateway"))
37+
? ok(`${HOST_GATEWAY_PID}\n`)
38+
: notFound()
39+
: command === "ps"
40+
? (psResults.get(args.at(-1) ?? "") ?? notFound())
41+
: command === "openshell" && args.join(" ") === "gateway list -o json"
42+
? ok("[]")
43+
: ok();
44+
try {
45+
const result = runUninstallPlan(
46+
{ assumeYes: true, deleteModels: false, keepOpenShell: false },
47+
{
48+
commandExists: (command) => command === "pgrep" || command === "openshell",
49+
env: { HOME: tmpHome, NO_COLOR: "1" },
50+
error: (message) => errors.push(message),
51+
existsSync: () => false,
52+
isTty: false,
53+
kill: () => false,
54+
log: vi.fn(),
55+
requireCompleteGatewayProcessCleanup: true,
56+
resolveGatewayTeardownAuthority: ({ gatewayName, gatewayPort }) => ({
57+
gatewayName,
58+
gatewayPort,
59+
mode: "nemoclaw-managed",
60+
source: "standalone",
61+
endpoint: null,
62+
stateDir: null,
63+
supervisor: null,
64+
requiredCapabilities: [],
65+
}),
66+
rmSync: vi.fn(),
67+
run,
68+
runDocker: () => ok(),
69+
},
70+
);
71+
return { errors, exitCode: result.exitCode };
72+
} finally {
73+
fs.rmSync(tmpHome, { force: true, recursive: true });
74+
}
75+
}
76+
77+
describe("uninstall with a host gateway owned by another user", () => {
78+
it("completes when the only unstoppable gateway process belongs to another user", () => {
79+
const { errors, exitCode } = uninstallWithHostGatewayOwnedBy((process.getuid?.() ?? 0) + 1);
80+
81+
expect(exitCode).toBe(0);
82+
expect(errors).toContainEqual(
83+
`Kept otheruser-owned host openshell-gateway process ${HOST_GATEWAY_PID} running. ` +
84+
"Cleanup does not stop a gateway process that another user owns.",
85+
);
86+
expect(errors).not.toContainEqual(
87+
"Cannot continue uninstall because host gateway process cleanup did not complete.",
88+
);
89+
});
90+
91+
it("still fails when the current user's own gateway process cannot be stopped", () => {
92+
const { errors, exitCode } = uninstallWithHostGatewayOwnedBy(process.getuid?.() ?? 0);
93+
94+
expect(exitCode).toBe(1);
95+
expect(errors).toContainEqual(
96+
"Cannot continue uninstall because host gateway process cleanup did not complete.",
97+
);
98+
});
99+
});

src/lib/onboard/host-gateway-process.test.ts

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ function psResponses(
5656
cmdline?: string;
5757
exited: Set<number>;
5858
owner?: string;
59+
uid?: number;
5960
},
6061
): [string, RunResult | ((args: string[]) => RunResult)][] {
6162
return [
@@ -65,9 +66,16 @@ function psResponses(
6566
`ps -p ${pid} -o args=`,
6667
ok(opts.cmdline ?? `/home/test/.local/bin/openshell-gateway --port 8080\n`),
6768
],
69+
...(opts.uid === undefined
70+
? []
71+
: [[`ps -p ${pid} -o uid=`, ok(`${opts.uid}\n`)] as [string, RunResult]]),
6872
];
6973
}
7074

75+
function otherUserUid(): number {
76+
return (process.getuid?.() ?? 0) + 1;
77+
}
78+
7179
describe("host gateway cleanup boundaries", () => {
7280
it.each([
7381
["free", 0, true],
@@ -350,7 +358,7 @@ describe("stopHostGatewayProcesses", () => {
350358
it("prints sudo remediation when a privileged host gateway cannot be killed", () => {
351359
const responses = new Map<string, RunResult | ((args: string[]) => RunResult)>([
352360
[PGREP_KEY, ok("9999042\n")],
353-
...psResponses(9999042, { exited: new Set(), owner: "root" }),
361+
...psResponses(9999042, { exited: new Set(), owner: "root", uid: 0 }),
354362
]);
355363
const { run } = makeRun(responses);
356364
const warn = vi.fn();
@@ -377,6 +385,93 @@ describe("stopHostGatewayProcesses", () => {
377385
);
378386
});
379387

388+
it("leaves a swept host gateway owned by another user running without failing cleanup", () => {
389+
const responses = new Map<string, RunResult | ((args: string[]) => RunResult)>([
390+
[PGREP_KEY, ok("9999043\n")],
391+
...psResponses(9999043, { exited: new Set(), owner: "otheruser", uid: otherUserUid() }),
392+
]);
393+
const { run } = makeRun(responses);
394+
const kill = vi.fn(() => false);
395+
const warn = vi.fn();
396+
397+
const result = stopHostGatewayProcesses(
398+
{
399+
run,
400+
kill,
401+
env: { USER: "tester" },
402+
commandExists: () => true,
403+
warn,
404+
},
405+
{
406+
killWaitMs: 0,
407+
stateDir: fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-host-gateway-")),
408+
termWaitMs: 0,
409+
},
410+
);
411+
412+
expect(result.foreignUserPids).toEqual([9999043]);
413+
expect(result.failed).toEqual([]);
414+
expect(result.sudoRemediationPids).toEqual([]);
415+
expect(result.stopped).toEqual([]);
416+
expect(kill).not.toHaveBeenCalled();
417+
expect(warn).toHaveBeenCalledWith(
418+
"Kept otheruser-owned host openshell-gateway process 9999043 running. " +
419+
"Cleanup does not stop a gateway process that another user owns.",
420+
);
421+
});
422+
423+
it("stops a foreign-user gateway recorded by this installation", () => {
424+
const pid = 9999045;
425+
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-host-gateway-"));
426+
fs.writeFileSync(path.join(stateDir, "openshell-gateway.pid"), `${pid}\n`);
427+
const exited = new Set<number>();
428+
const responses = new Map<string, RunResult | ((args: string[]) => RunResult)>([
429+
[PGREP_KEY, notFound()],
430+
...psResponses(pid, { exited, owner: "otheruser", uid: otherUserUid() }),
431+
]);
432+
const { run } = makeRun(responses);
433+
const kill = vi.fn<HostGatewayProcessDeps["kill"]>((targetPid, signal) => {
434+
signal === "SIGTERM" && exited.add(targetPid);
435+
return true;
436+
});
437+
438+
const result = stopHostGatewayProcesses(
439+
{ run, kill, env: { USER: "tester" }, commandExists: () => true, log: vi.fn() },
440+
{ stateDir },
441+
);
442+
443+
expect(result.stopped).toEqual([pid]);
444+
expect(result.foreignUserPids).toEqual([]);
445+
expect(kill).toHaveBeenCalledWith(pid, "SIGTERM");
446+
});
447+
448+
it("still stops a swept host gateway owned by the current user", () => {
449+
const exited = new Set<number>();
450+
const responses = new Map<string, RunResult | ((args: string[]) => RunResult)>([
451+
[PGREP_KEY, ok("9999044\n")],
452+
...psResponses(9999044, { exited, uid: process.getuid?.() ?? 0 }),
453+
]);
454+
const { run } = makeRun(responses);
455+
const kill = vi.fn<HostGatewayProcessDeps["kill"]>((pid, signal) => {
456+
signal === "SIGTERM" && exited.add(pid);
457+
return true;
458+
});
459+
460+
const result = stopHostGatewayProcesses(
461+
{
462+
run,
463+
kill,
464+
env: { USER: "tester" },
465+
commandExists: () => true,
466+
log: vi.fn(),
467+
},
468+
{ stateDir: fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-host-gateway-")) },
469+
);
470+
471+
expect(result.stopped).toEqual([9999044]);
472+
expect(result.foreignUserPids).toEqual([]);
473+
});
474+
380475
it("skips pgrep sweep when explicit PIDs are passed (drift restart)", () => {
381476
// Use a PID above the Linux kernel pid_max default (4194304) so that the
382477
// production code's `/proc/<pid>/cmdline` probe always misses and the

src/lib/onboard/host-gateway-process.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ export interface StopHostGatewayOptions {
6666

6767
export interface StopHostGatewayResult {
6868
failed: number[];
69+
foreignUserPids?: number[];
6970
/** Whether a requested pgrep fallback completed with a usable result. */
7071
orphanScanComplete?: boolean;
7172
ownershipFailures?: string[];
@@ -204,6 +205,31 @@ function pidOwner(pid: number, deps: HostGatewayProcessDeps): string | null {
204205
return result.stdout.trim() || null;
205206
}
206207

208+
function pidOwnerUid(pid: number, deps: HostGatewayProcessDeps): number | null {
209+
const result = deps.run("ps", ["-p", String(pid), "-o", "uid="], { env: deps.env });
210+
if (result.status !== 0) return null;
211+
const uid = Number.parseInt(result.stdout.trim(), 10);
212+
return Number.isInteger(uid) ? uid : null;
213+
}
214+
215+
function pidBelongsToAnotherUser(pid: number, deps: HostGatewayProcessDeps): boolean {
216+
const currentUid = typeof process.getuid === "function" ? process.getuid() : -1;
217+
if (currentUid < 0) return false;
218+
const uid = pidOwnerUid(pid, deps);
219+
if (uid === null || uid === 0) return false;
220+
return uid !== currentUid;
221+
}
222+
223+
function warnForeignUserGateway(pid: number, deps: HostGatewayProcessDeps): void {
224+
const warn = deps.warn ?? ((message: string) => console.warn(message));
225+
const owner = pidOwner(pid, deps);
226+
const ownerLabel = owner ? `${owner}-owned` : "another user's";
227+
warn(
228+
`Kept ${ownerLabel} host openshell-gateway process ${pid} running. ` +
229+
"Cleanup does not stop a gateway process that another user owns.",
230+
);
231+
}
232+
207233
function readOwnedRuntimeFile(filePath: string, uid: number): string | null {
208234
if (typeof fs.constants.O_NOFOLLOW !== "number") return null;
209235
let descriptor: number | undefined;
@@ -417,6 +443,7 @@ export function stopHostGatewayProcesses(
417443
const candidates = new Map<number, Set<string>>();
418444
const result: StopHostGatewayResult = {
419445
failed: [],
446+
foreignUserPids: [],
420447
orphanScanComplete: true,
421448
ownershipFailures: [],
422449
skippedDeadPids: [],
@@ -546,6 +573,15 @@ export function stopHostGatewayProcesses(
546573
}
547574
continue;
548575
}
576+
if (
577+
!options.scopedGatewayStop &&
578+
!sources.has("pid-file") &&
579+
pidBelongsToAnotherUser(pid, deps)
580+
) {
581+
(result.foreignUserPids ??= []).push(pid);
582+
warnForeignUserGateway(pid, deps);
583+
continue;
584+
}
549585

550586
const stopResult = tryStopPid(
551587
pid,

0 commit comments

Comments
 (0)