mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 20:05:46 -06:00
fix: allow gateway service commands for named profiles (#116314)
* fix: gateway service commands refuse a named profile or relocated OPENCLAW_HOME - Resolve the default install identity against the canonical state directory for the active OpenClaw home and profile instead of the unprofiled OS account default. - `--profile <name>` / `--dev` project `.openclaw-<profile>` state and config paths, so every named profile was classified as isolated state and refused `install`, `start`, `stop`, `restart`, `uninstall`, Doctor service repair, and self-update service handling. - `OPENCLAW_HOME` relocates all OpenClaw path defaults and is documented for running as a dedicated service user; a relocated home is now an install identity. `HOME` alone still is not. - An `OPENCLAW_STATE_DIR` or `OPENCLAW_CONFIG_PATH` pointing outside those canonical paths is still treated as isolated state. - Recovery guidance in the refusal message now names the paths that must match. Verified: focused vitest shards for the changed suites plus the daemon, CLI, and doctor suites that consume the identity check; tsgo core and core-test lanes; oxlint; docs format, MDX, link, and map checks. * fix(gateway): keep relocated homes isolated * fix(config): validate service profile identity * fix(daemon): enforce named-profile service ownership * fix(update): reject drifted service selectors before probes * test(windows): prove scheduled task lifecycle * test(windows): harden scheduled task proof cleanup * test(windows): bind lifecycle proof to checkout * test(windows): normalize cleanup exit status * test(windows): verify effective task privilege * test(windows): protect scheduled task proof roots * test(windows): prove listener-owned task lifecycle * test(windows): fix scheduled task proof contracts * test(windows): remove redundant mock coercions * test(windows): measure fallback before task probes * test(windows): prove scheduled task process origin * fix(gateway): preserve unmanaged restart fallback * test(gateway): cover denied restart ownership * test(gateway): keep restart helper types private * test(gateway): classify lifecycle helpers as test code --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -38,9 +38,13 @@ import {
|
||||
} from "./update-command-post-core.js";
|
||||
import { POST_PLUGIN_DOCTOR_EXECUTION_FAILED_REASON } from "./update-command-post-plugin-validation.js";
|
||||
import {
|
||||
assertGatewayServiceManagementAllowedForUpdate,
|
||||
GatewayServiceUpdateOwnershipError,
|
||||
gatewayServiceCommandUsesRoot,
|
||||
isGatewayServiceManagementAllowedForUpdate,
|
||||
maybeRestartService,
|
||||
maybeRestartServiceAfterFailedMutableUpdate,
|
||||
resolveGatewayServiceManagementBlockMessageForUpdate,
|
||||
resolvePostUpdateServiceStateReadEnv,
|
||||
resolveUpdatedGatewayRestartPort,
|
||||
restoreWindowsTaskAutoStartOrExit,
|
||||
@@ -340,18 +344,30 @@ export async function finishUpdate(params: {
|
||||
let refreshGatewayServiceEnv = false;
|
||||
let gatewayServiceEnv: NodeJS.ProcessEnv | undefined;
|
||||
let skipLegacyServiceRestart = false;
|
||||
const serviceStateReadEnv = resolvePostUpdateServiceStateReadEnv({
|
||||
updateMode: resultWithPostUpdate.mode,
|
||||
processEnv: process.env,
|
||||
preManagedServiceEnv: params.preManagedServiceStop?.serviceEnv,
|
||||
});
|
||||
const serviceMutationAllowed =
|
||||
params.preManagedServiceStop?.serviceMutationAllowed !== false &&
|
||||
isGatewayServiceManagementAllowedForUpdate(process.env) &&
|
||||
isGatewayServiceManagementAllowedForUpdate(serviceStateReadEnv);
|
||||
const serviceMutationSkipMessage =
|
||||
params.shouldRestart && !serviceMutationAllowed
|
||||
? (params.preManagedServiceStop?.serviceMutationSkipMessage ??
|
||||
resolveGatewayServiceManagementBlockMessageForUpdate(process.env) ??
|
||||
resolveGatewayServiceManagementBlockMessageForUpdate(serviceStateReadEnv))
|
||||
: undefined;
|
||||
let gatewayPort = resolveUpdatedGatewayRestartPort({
|
||||
config: restartConfigSnapshot.valid ? restartConfigSnapshot.config : undefined,
|
||||
processEnv: process.env,
|
||||
});
|
||||
if (params.shouldRestart) {
|
||||
if (params.shouldRestart && serviceMutationAllowed) {
|
||||
try {
|
||||
const serviceState = await readGatewayServiceState(resolveGatewayService(), {
|
||||
env: resolvePostUpdateServiceStateReadEnv({
|
||||
updateMode: resultWithPostUpdate.mode,
|
||||
processEnv: process.env,
|
||||
preManagedServiceEnv: params.preManagedServiceStop?.serviceEnv,
|
||||
}),
|
||||
env: serviceStateReadEnv,
|
||||
validateEnvBeforeStatusRead: assertGatewayServiceManagementAllowedForUpdate,
|
||||
});
|
||||
const serviceMatchesUpdateRoot =
|
||||
(await gatewayServiceCommandUsesRoot({
|
||||
@@ -399,7 +415,12 @@ export async function finishUpdate(params: {
|
||||
// ownership authorizes rewriting the service definition.
|
||||
refreshGatewayServiceEnv = serviceOwnershipConfirmed;
|
||||
}
|
||||
} catch {
|
||||
} catch (err) {
|
||||
if (err instanceof GatewayServiceUpdateOwnershipError) {
|
||||
defaultRuntime.error(err.message);
|
||||
defaultRuntime.exit(1);
|
||||
return;
|
||||
}
|
||||
// Ignore errors during pre-check; fallback to standard restart
|
||||
}
|
||||
}
|
||||
@@ -420,7 +441,7 @@ export async function finishUpdate(params: {
|
||||
return;
|
||||
}
|
||||
const restartOk = await maybeRestartService({
|
||||
shouldRestart: params.shouldRestart,
|
||||
shouldRestart: params.shouldRestart && serviceMutationAllowed,
|
||||
result: resultWithPostUpdate,
|
||||
opts: params.opts,
|
||||
refreshServiceEnv: refreshGatewayServiceEnv,
|
||||
@@ -432,6 +453,7 @@ export async function finishUpdate(params: {
|
||||
skipLegacyServiceRestart,
|
||||
requireRunningServiceAfterRestart:
|
||||
resultWithPostUpdate.mode === "git" && params.preManagedServiceStop?.stopped === true,
|
||||
serviceMutationSkipMessage,
|
||||
timeoutMs: params.updateStepTimeoutMs,
|
||||
});
|
||||
if (!restartOk) {
|
||||
|
||||
@@ -29,6 +29,7 @@ import {
|
||||
import { summarizeGatewayServiceLayout } from "../../daemon/service-layout.js";
|
||||
import type { GatewayServiceCommandConfig } from "../../daemon/service-types.js";
|
||||
import { readGatewayServiceState, resolveGatewayService } from "../../daemon/service.js";
|
||||
import { assertGatewayServiceMutationAllowed } from "../../infra/gateway-supervision.js";
|
||||
import { parseStrictPositiveInteger } from "../../infra/parse-finite-number.js";
|
||||
import { getSelfAndAncestorPidsSync } from "../../infra/restart-stale-pids.js";
|
||||
import { nodeVersionSatisfiesEngine } from "../../infra/runtime-guard.js";
|
||||
@@ -111,6 +112,8 @@ export type PreManagedServiceStop = {
|
||||
inspected: boolean;
|
||||
runtimeInspected: boolean;
|
||||
running: boolean;
|
||||
serviceMutationAllowed?: boolean;
|
||||
serviceMutationSkipMessage?: string;
|
||||
serviceMatchesMutationRoot?: boolean;
|
||||
blockMessage?: string;
|
||||
serviceEnv?: NodeJS.ProcessEnv;
|
||||
@@ -128,6 +131,41 @@ export type UpdateCommandRecoveryState = {
|
||||
windowsTaskAutoStartRecovery?: WindowsTaskAutoStartRecovery;
|
||||
};
|
||||
|
||||
export class GatewayServiceUpdateOwnershipError extends Error {
|
||||
constructor(message: string, cause: unknown) {
|
||||
super(message, { cause });
|
||||
this.name = "GatewayServiceUpdateOwnershipError";
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveGatewayServiceManagementBlockMessageForUpdate(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): string | undefined {
|
||||
try {
|
||||
assertGatewayServiceManagementAllowedForUpdate(env);
|
||||
return undefined;
|
||||
} catch (err) {
|
||||
return err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
}
|
||||
|
||||
export function assertGatewayServiceManagementAllowedForUpdate(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): void {
|
||||
try {
|
||||
assertGatewayServiceMutationAllowed("manage the gateway service during update", env);
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
throw new GatewayServiceUpdateOwnershipError(message, err);
|
||||
}
|
||||
}
|
||||
|
||||
export function isGatewayServiceManagementAllowedForUpdate(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): boolean {
|
||||
return resolveGatewayServiceManagementBlockMessageForUpdate(env) === undefined;
|
||||
}
|
||||
|
||||
export class UpdateCommandAbort extends Error {
|
||||
constructor() {
|
||||
super("openclaw-update-abort");
|
||||
@@ -334,12 +372,38 @@ export async function maybeStopManagedServiceBeforeMutableUpdate(params: {
|
||||
shouldRestart: boolean;
|
||||
jsonMode: boolean;
|
||||
}): Promise<PreManagedServiceStop> {
|
||||
const serviceMutationSkipMessage = resolveGatewayServiceManagementBlockMessageForUpdate(
|
||||
process.env,
|
||||
);
|
||||
if (serviceMutationSkipMessage) {
|
||||
return {
|
||||
stopped: false,
|
||||
inspected: false,
|
||||
runtimeInspected: false,
|
||||
running: false,
|
||||
serviceMutationAllowed: false,
|
||||
serviceMutationSkipMessage,
|
||||
};
|
||||
}
|
||||
let service: ReturnType<typeof resolveGatewayService>;
|
||||
let serviceState: Awaited<ReturnType<typeof readGatewayServiceState>>;
|
||||
try {
|
||||
service = resolveGatewayService();
|
||||
serviceState = await readGatewayServiceState(service, { env: process.env });
|
||||
} catch {
|
||||
serviceState = await readGatewayServiceState(service, {
|
||||
env: process.env,
|
||||
validateEnvBeforeStatusRead: assertGatewayServiceManagementAllowedForUpdate,
|
||||
});
|
||||
} catch (err) {
|
||||
if (err instanceof GatewayServiceUpdateOwnershipError) {
|
||||
return {
|
||||
stopped: false,
|
||||
inspected: false,
|
||||
runtimeInspected: false,
|
||||
running: false,
|
||||
serviceMutationAllowed: false,
|
||||
blockMessage: err.message,
|
||||
};
|
||||
}
|
||||
return { stopped: false, inspected: false, runtimeInspected: false, running: false };
|
||||
}
|
||||
|
||||
@@ -949,6 +1013,9 @@ function resolveManagedServiceNodeRunner(
|
||||
* when the package root is the same.
|
||||
*/
|
||||
export async function resolveManagedServiceNodeRunnerOverride(): Promise<string | undefined> {
|
||||
if (!isGatewayServiceManagementAllowedForUpdate(process.env)) {
|
||||
return undefined;
|
||||
}
|
||||
const command = await resolveGatewayService()
|
||||
.readCommand(process.env)
|
||||
.catch(() => null);
|
||||
@@ -970,6 +1037,9 @@ export async function resolveManagedServiceNodeRunnerOverride(): Promise<string
|
||||
export async function resolveManagedServicePackageUpdateRoot(params: {
|
||||
root: string;
|
||||
}): Promise<ManagedServiceRootRedirect | null> {
|
||||
if (!isGatewayServiceManagementAllowedForUpdate(process.env)) {
|
||||
return null;
|
||||
}
|
||||
const command = await resolveGatewayService()
|
||||
.readCommand(process.env)
|
||||
.catch(() => null);
|
||||
@@ -1004,9 +1074,11 @@ export async function gatewayServiceCommandUsesRoot(params: {
|
||||
}
|
||||
const command =
|
||||
params.command === undefined
|
||||
? await resolveGatewayService()
|
||||
.readCommand(params.env ?? process.env)
|
||||
.catch(() => null)
|
||||
? isGatewayServiceManagementAllowedForUpdate(params.env ?? process.env)
|
||||
? await resolveGatewayService()
|
||||
.readCommand(params.env ?? process.env)
|
||||
.catch(() => null)
|
||||
: null
|
||||
: params.command;
|
||||
const layout = await summarizeGatewayServiceLayout(command);
|
||||
const serviceRoot = layout?.packageRoot;
|
||||
@@ -1037,8 +1109,22 @@ export async function maybeRestartService(params: {
|
||||
nodeRunner?: string;
|
||||
skipLegacyServiceRestart?: boolean;
|
||||
requireRunningServiceAfterRestart?: boolean;
|
||||
serviceMutationSkipMessage?: string;
|
||||
timeoutMs: number;
|
||||
}): Promise<boolean> {
|
||||
if (
|
||||
params.shouldRestart &&
|
||||
(!isGatewayServiceManagementAllowedForUpdate(process.env) ||
|
||||
!isGatewayServiceManagementAllowedForUpdate(params.serviceEnv ?? process.env))
|
||||
) {
|
||||
const message =
|
||||
resolveGatewayServiceManagementBlockMessageForUpdate(process.env) ??
|
||||
resolveGatewayServiceManagementBlockMessageForUpdate(params.serviceEnv ?? process.env);
|
||||
if (message) {
|
||||
defaultRuntime.error(message);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
const verifyRestartedGateway = async (
|
||||
expectedGatewayVersion: string | undefined,
|
||||
opts: { requireRunningService?: boolean } = {},
|
||||
@@ -1325,6 +1411,18 @@ export async function maybeRestartService(params: {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (params.serviceMutationSkipMessage) {
|
||||
if (params.opts.json) {
|
||||
defaultRuntime.error(params.serviceMutationSkipMessage);
|
||||
} else {
|
||||
defaultRuntime.log("");
|
||||
defaultRuntime.log(
|
||||
theme.warn(`Gateway: restart skipped: ${params.serviceMutationSkipMessage}`),
|
||||
);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!params.opts.json) {
|
||||
defaultRuntime.log("");
|
||||
defaultRuntime.log(theme.muted("Gateway: restart skipped (--no-restart)."));
|
||||
|
||||
@@ -6,6 +6,7 @@ import { describe, expect, it, vi } from "vitest";
|
||||
import { resolveGatewayInstallEntrypoint } from "../../daemon/gateway-entrypoint.js";
|
||||
import type { GatewayService } from "../../daemon/service.js";
|
||||
import type { UpdateRunResult } from "../../infra/update-runner.js";
|
||||
import { defaultRuntime } from "../../runtime.js";
|
||||
import {
|
||||
updatePluginsAfterCoreUpdate,
|
||||
type PostCorePluginUpdateResult,
|
||||
@@ -17,6 +18,7 @@ import {
|
||||
resolvePostInstallDoctorEnv,
|
||||
resolvePostUpdateServiceStateReadEnv,
|
||||
resolveUpdatedGatewayRestartPort,
|
||||
maybeRestartService,
|
||||
shouldPrepareUpdatedInstallRestart,
|
||||
} from "./update-command-service.js";
|
||||
import { testing as updateCommandServiceTesting } from "./update-command-service.test-support.js";
|
||||
@@ -176,6 +178,31 @@ describe("resolveUpdatedGatewayRestartPort", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("maybeRestartService", () => {
|
||||
it("reports service ownership skips to JSON callers", async () => {
|
||||
const errorSpy = vi.spyOn(defaultRuntime, "error").mockImplementation(() => undefined);
|
||||
|
||||
await expect(
|
||||
maybeRestartService({
|
||||
shouldRestart: false,
|
||||
result: {
|
||||
status: "ok",
|
||||
mode: "npm",
|
||||
steps: [],
|
||||
durationMs: 0,
|
||||
},
|
||||
opts: { json: true },
|
||||
refreshServiceEnv: false,
|
||||
gatewayPort: 18789,
|
||||
serviceMutationSkipMessage: "service management skipped: ownership conflict",
|
||||
timeoutMs: 1_000,
|
||||
}),
|
||||
).resolves.toBe(true);
|
||||
|
||||
expect(errorSpy).toHaveBeenCalledWith("service management skipped: ownership conflict");
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolvePostUpdateServiceStateReadEnv", () => {
|
||||
it("keeps package restart preparation anchored to the pre-update service env", () => {
|
||||
const processEnv = {
|
||||
|
||||
Reference in New Issue
Block a user