fixes for cli-containerized (#54223)

Signed-off-by: sallyom <somalley@redhat.com>
This commit is contained in:
Sally O'Malley
2026-03-25 00:51:55 -04:00
committed by GitHub
parent 1c9f62fad3
commit e5d0d810e1
6 changed files with 76 additions and 139 deletions

View File

@@ -162,7 +162,7 @@ describe("maybeRunCliInContainer", () => {
);
});
it("clears inherited OPENCLAW_CONTAINER before execing into the child CLI", () => {
it("clears inherited host routing and gateway env before execing into the child CLI", () => {
const spawnSync = vi
.fn()
.mockReturnValueOnce({
@@ -181,7 +181,11 @@ describe("maybeRunCliInContainer", () => {
maybeRunCliInContainer(["node", "openclaw", "status"], {
env: {
OPENCLAW_CONTAINER: "demo",
OPENCLAW_PROFILE: "work",
OPENCLAW_GATEWAY_PORT: "19001",
OPENCLAW_GATEWAY_URL: "ws://127.0.0.1:18789",
OPENCLAW_GATEWAY_TOKEN: "token",
OPENCLAW_GATEWAY_PASSWORD: "password",
} as NodeJS.ProcessEnv,
spawnSync,
});
@@ -204,7 +208,6 @@ describe("maybeRunCliInContainer", () => {
stdio: "inherit",
env: {
OPENCLAW_CONTAINER: "",
OPENCLAW_GATEWAY_TOKEN: "token",
},
},
);
@@ -316,67 +319,7 @@ describe("maybeRunCliInContainer", () => {
);
});
it("falls back to sudo -u openclaw podman for the documented dedicated-user flow", () => {
const spawnSync = vi
.fn()
.mockReturnValueOnce({
status: 1,
stdout: "",
})
.mockReturnValueOnce({
status: 1,
stdout: "",
})
.mockReturnValueOnce({
status: 0,
stdout: "true\n",
})
.mockReturnValueOnce({
status: 0,
stdout: "",
});
expect(
maybeRunCliInContainer(["node", "openclaw", "--container", "openclaw", "status"], {
env: { USER: "somalley" } as NodeJS.ProcessEnv,
spawnSync,
}),
).toEqual({
handled: true,
exitCode: 0,
});
expect(spawnSync).toHaveBeenNthCalledWith(
3,
"sudo",
["-u", "openclaw", "podman", "inspect", "--format", "{{.State.Running}}", "openclaw"],
{ encoding: "utf8", stdio: ["inherit", "pipe", "inherit"] },
);
expect(spawnSync).toHaveBeenNthCalledWith(
4,
"sudo",
[
"-u",
"openclaw",
"podman",
"exec",
"-i",
"--env",
"OPENCLAW_CONTAINER_HINT=openclaw",
"--env",
"OPENCLAW_CLI_CONTAINER_BYPASS=1",
"openclaw",
"openclaw",
"status",
],
{
stdio: "inherit",
env: { USER: "somalley", OPENCLAW_CONTAINER: "" },
},
);
});
it("checks docker before the dedicated-user podman fallback", () => {
it("checks docker after podman and before failing", () => {
const spawnSync = vi
.fn()
.mockReturnValueOnce({
@@ -440,6 +383,40 @@ describe("maybeRunCliInContainer", () => {
expect(spawnSync).toHaveBeenCalledTimes(3);
});
it("does not try any sudo podman fallback for regular users", () => {
const spawnSync = vi
.fn()
.mockReturnValueOnce({
status: 1,
stdout: "",
})
.mockReturnValueOnce({
status: 1,
stdout: "",
});
expect(() =>
maybeRunCliInContainer(["node", "openclaw", "--container", "demo", "status"], {
env: { USER: "somalley" } as NodeJS.ProcessEnv,
spawnSync,
}),
).toThrow('No running container matched "demo" under podman or docker.');
expect(spawnSync).toHaveBeenCalledTimes(2);
expect(spawnSync).toHaveBeenNthCalledWith(
1,
"podman",
["inspect", "--format", "{{.State.Running}}", "demo"],
{ encoding: "utf8" },
);
expect(spawnSync).toHaveBeenNthCalledWith(
2,
"docker",
["inspect", "--format", "{{.State.Running}}", "demo"],
{ encoding: "utf8" },
);
});
it("rejects ambiguous matches across runtimes", () => {
const spawnSync = vi
.fn()

View File

@@ -100,8 +100,8 @@ function isContainerRunning(params: {
return result.status === 0 && result.stdout.trim() === "true";
}
function candidateContainerRuntimes(env: NodeJS.ProcessEnv): ContainerRuntimeExec[] {
const candidates: ContainerRuntimeExec[] = [
function candidateContainerRuntimes(): ContainerRuntimeExec[] {
return [
{
runtime: "podman",
command: "podman",
@@ -113,24 +113,6 @@ function candidateContainerRuntimes(env: NodeJS.ProcessEnv): ContainerRuntimeExe
argsPrefix: [],
},
];
const podmanUser = env.OPENCLAW_PODMAN_USER?.trim() || "openclaw";
const currentUser = env.USER?.trim() || env.LOGNAME?.trim() || "";
if (podmanUser && currentUser && podmanUser !== currentUser) {
candidates.push({
runtime: "podman",
command: "sudo",
argsPrefix: ["-u", podmanUser, "podman"],
});
}
return candidates;
}
function describeContainerRuntimeExec(exec: ContainerRuntimeExec): string {
if (exec.command === "sudo") {
const podmanUser = exec.argsPrefix[1];
return `podman (via sudo -u ${podmanUser})`;
}
return exec.runtime;
}
function resolveRunningContainer(params: {
@@ -139,7 +121,7 @@ function resolveRunningContainer(params: {
deps: Pick<ContainerTargetDeps, "spawnSync">;
}): (ContainerRuntimeExec & { containerName: string }) | null {
const matches: Array<ContainerRuntimeExec & { containerName: string }> = [];
const candidates = candidateContainerRuntimes(params.env);
const candidates = candidateContainerRuntimes();
for (const exec of candidates) {
if (
isContainerRunning({
@@ -158,7 +140,7 @@ function resolveRunningContainer(params: {
return null;
}
if (matches.length > 1) {
const runtimes = matches.map(describeContainerRuntimeExec).join(", ");
const runtimes = matches.map((match) => match.runtime).join(", ");
throw new Error(
`Container "${params.containerName}" is running under multiple runtimes (${runtimes}); use a unique container name.`,
);
@@ -190,13 +172,19 @@ function buildContainerExecArgs(params: {
}
function buildContainerExecEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
return {
...env,
// The child CLI should render container-aware follow-up commands via
// OPENCLAW_CONTAINER_HINT, but it should not treat itself as still
// container-targeted for validation/routing.
OPENCLAW_CONTAINER: "",
};
const next = { ...env };
// Container-targeted CLI invocations should use the container's own profile
// and gateway auth/runtime state rather than inheriting host overrides.
delete next.OPENCLAW_PROFILE;
delete next.OPENCLAW_GATEWAY_PORT;
delete next.OPENCLAW_GATEWAY_URL;
delete next.OPENCLAW_GATEWAY_TOKEN;
delete next.OPENCLAW_GATEWAY_PASSWORD;
// The child CLI should render container-aware follow-up commands via
// OPENCLAW_CONTAINER_HINT, but it should not treat itself as still
// container-targeted for validation/routing.
next.OPENCLAW_CONTAINER = "";
return next;
}
function isBlockedContainerCommand(argv: string[]): boolean {

View File

@@ -131,9 +131,7 @@ describe("runCli profile env bootstrap", () => {
it("rejects --container combined with --profile", async () => {
await expect(
runCli(["node", "openclaw", "--container", "demo", "--profile", "rawdog", "status"]),
).rejects.toThrow(
"--container cannot be combined with --profile/--dev or gateway override env vars",
);
).rejects.toThrow("--container cannot be combined with --profile/--dev");
expect(dotenvState.loadDotEnv).not.toHaveBeenCalled();
expect(process.env.OPENCLAW_PROFILE).toBe("rawdog");
@@ -142,17 +140,13 @@ describe("runCli profile env bootstrap", () => {
it("rejects --container combined with interleaved --profile", async () => {
await expect(
runCli(["node", "openclaw", "status", "--container", "demo", "--profile", "rawdog"]),
).rejects.toThrow(
"--container cannot be combined with --profile/--dev or gateway override env vars",
);
).rejects.toThrow("--container cannot be combined with --profile/--dev");
});
it("rejects --container combined with interleaved --dev", async () => {
await expect(
runCli(["node", "openclaw", "status", "--container", "demo", "--dev"]),
).rejects.toThrow(
"--container cannot be combined with --profile/--dev or gateway override env vars",
);
).rejects.toThrow("--container cannot be combined with --profile/--dev");
});
it("does not let dotenv change container target resolution", async () => {
@@ -174,12 +168,12 @@ describe("runCli profile env bootstrap", () => {
});
});
it("rejects container mode when OPENCLAW_PROFILE is already set in env", async () => {
it("allows container mode when OPENCLAW_PROFILE is already set in env", async () => {
process.env.OPENCLAW_PROFILE = "work";
await expect(runCli(["node", "openclaw", "--container", "demo", "status"])).rejects.toThrow(
"--container cannot be combined with --profile/--dev or gateway override env vars",
);
await expect(
runCli(["node", "openclaw", "--container", "demo", "status"]),
).resolves.toBeUndefined();
});
it.each([
@@ -187,12 +181,12 @@ describe("runCli profile env bootstrap", () => {
["OPENCLAW_GATEWAY_URL", "ws://127.0.0.1:18789"],
["OPENCLAW_GATEWAY_TOKEN", "demo-token"],
["OPENCLAW_GATEWAY_PASSWORD", "demo-password"],
])("rejects container mode when %s is set in env", async (key, value) => {
])("allows container mode when %s is set in env", async (key, value) => {
process.env[key] = value;
await expect(runCli(["node", "openclaw", "--container", "demo", "status"])).rejects.toThrow(
"--container cannot be combined with --profile/--dev or gateway override env vars",
);
await expect(
runCli(["node", "openclaw", "--container", "demo", "status"]),
).resolves.toBeUndefined();
});
it("allows container mode when only OPENCLAW_STATE_DIR is set in env", async () => {

View File

@@ -95,18 +95,8 @@ export async function runCli(argv: string[] = process.argv) {
}
const containerTargetName =
parsedContainer.container ?? process.env.OPENCLAW_CONTAINER?.trim() ?? null;
if (
containerTargetName &&
(parsedProfile.profile ||
process.env.OPENCLAW_PROFILE?.trim() ||
process.env.OPENCLAW_GATEWAY_PORT?.trim() ||
process.env.OPENCLAW_GATEWAY_URL?.trim() ||
process.env.OPENCLAW_GATEWAY_TOKEN?.trim() ||
process.env.OPENCLAW_GATEWAY_PASSWORD?.trim())
) {
throw new Error(
"--container cannot be combined with --profile/--dev or gateway override env vars",
);
if (containerTargetName && parsedProfile.profile) {
throw new Error("--container cannot be combined with --profile/--dev");
}
const containerTarget = maybeRunCliInContainer(originalArgv);

View File

@@ -139,18 +139,8 @@ if (
}
const containerTargetName = resolveCliContainerTarget(process.argv);
if (
containerTargetName &&
(parsed.profile ||
process.env.OPENCLAW_PROFILE?.trim() ||
process.env.OPENCLAW_GATEWAY_PORT?.trim() ||
process.env.OPENCLAW_GATEWAY_URL?.trim() ||
process.env.OPENCLAW_GATEWAY_TOKEN?.trim() ||
process.env.OPENCLAW_GATEWAY_PASSWORD?.trim())
) {
console.error(
"[openclaw] --container cannot be combined with --profile/--dev or gateway override env vars",
);
if (containerTargetName && parsed.profile) {
console.error("[openclaw] --container cannot be combined with --profile/--dev");
process.exit(2);
}

View File

@@ -136,7 +136,7 @@ describe("entry root version fast path", () => {
logSpy.mockRestore();
});
it("rejects container mode for root version when gateway override env vars are set", async () => {
it("allows root version container mode when gateway override env vars are set", async () => {
resolveCliContainerTargetMock.mockReturnValue("demo");
process.env.OPENCLAW_GATEWAY_TOKEN = "demo-token";
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
@@ -144,12 +144,10 @@ describe("entry root version fast path", () => {
await import("./entry.js");
await vi.waitFor(() => {
expect(errorSpy).toHaveBeenCalledWith(
"[openclaw] --container cannot be combined with --profile/--dev or gateway override env vars",
);
expect(exitSpy).toHaveBeenCalledWith(2);
expect(runCliMock).toHaveBeenCalledWith(["node", "openclaw", "--version"]);
});
expect(runCliMock).not.toHaveBeenCalled();
expect(errorSpy).not.toHaveBeenCalled();
expect(exitSpy).not.toHaveBeenCalled();
errorSpy.mockRestore();
});