From cd9db5ed9aa386c92762ff82b31a372b62bf041e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 10 Jul 2026 05:42:36 +0100 Subject: [PATCH] fix(doctor): keep automated repair from moving approval state (#103353) * fix(doctor): isolate automated cross-state imports * test(update): cover isolated doctor finalization --- CHANGELOG.md | 2 + docs/tools/exec-approvals.md | 13 +-- .../e2e/lib/doctor-install-switch/scenario.sh | 83 +++++++++++++++++++ scripts/watch-node.mjs | 9 +- src/cli/program/config-guard.test.ts | 45 +++++++--- src/cli/program/config-guard.ts | 28 ++++++- src/cli/program/register.maintenance.test.ts | 29 ++++++- src/cli/program/register.maintenance.ts | 2 + src/cli/update-cli.test.ts | 2 + src/cli/update-cli/update-command.test.ts | 3 + src/cli/update-cli/update-command.ts | 9 +- src/commands/doctor-config-flow.test.ts | 29 ++++++- src/commands/doctor-config-flow.ts | 2 +- src/commands/doctor-invocation.ts | 16 ++++ src/commands/doctor.types.ts | 2 + src/flows/doctor-health-contributions.test.ts | 46 ++++++++++ src/flows/doctor-health-contributions.ts | 13 ++- src/infra/update-runner.test.ts | 2 + src/infra/update-runner.ts | 3 + src/infra/watch-node.test.ts | 6 +- 20 files changed, 314 insertions(+), 30 deletions(-) create mode 100644 src/commands/doctor-invocation.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4714f8e8bc72..62678c6edf05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ Docs: https://docs.openclaw.ai ### Fixes - **OpenAI-compatible streamed tool calls:** execute complete native tool calls from streams that end with SSE `data: [DONE]` but omit `finish_reason`, while keeping transport EOF and visible-text cases fail-closed. (#98124, #97994) Thanks @SunnyShu0925. +- **Doctor state isolation:** prevent automated update and Gateway watch repair from importing and archiving default-home exec or plugin-binding approvals when `OPENCLAW_STATE_DIR` points elsewhere, keep implicit CLI preflight notice-only, and reserve cross-state imports for direct operator doctor runs. (#103247, #103317) +- **Doctor clean-state guidance:** stop suggesting `openclaw doctor --fix` after a clean run with no config changes while preserving targeted repair hints. (#103233) - **OpenCode Zen model catalog:** refresh the provider-owned static seed for Claude Sonnet 5, Grok 4.5, Hy3 Free, Kimi K2.7 Code, and MiniMax M3 with verified routing, pricing, limits, and input capabilities, remove retired free-tier rows, and expose the same catalog through unauthenticated model listing. (#103184) - **Managed browser launch:** surface asynchronous Chrome bootstrap and runtime spawn failures as browser errors while keeping Gateway alive, and retain process error handling through later lifecycle failures. - **Browser node-proxy downloads:** transfer every action-produced download to the Gateway media store, align a 10 MiB per-file and 16 MiB aggregate transport budget, and rewrite plural download paths to Gateway-local files without traversing page-controlled result data. diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 5c6af0b0c564..b36a07e54002 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -89,12 +89,15 @@ The default approval socket follows the same root: `$OPENCLAW_STATE_DIR/exec-approvals.sock`, or `~/.openclaw/exec-approvals.sock` when the variable is unset. -Releases before 2026.6.11 always kept the file in `~/.openclaw`. If +Releases before 2026.6.6 always kept the file in `~/.openclaw`. If `OPENCLAW_STATE_DIR` points somewhere else and an approvals file still exists -in the default directory, run `openclaw doctor --fix` once to import it into -the state directory (the original is archived with a `.migrated` suffix). -OpenClaw never imports it automatically: a gateway pointed at a temporary or -staging state directory must not capture the default installation's approvals. +in the default directory, run `openclaw doctor --fix` directly once to import +it into the state directory (the original is archived with a `.migrated` +suffix). Interactive doctor can also preview and confirm the import. Automated +update and Gateway watch repair runs never import across state directories: a +temporary or staging state directory must not capture the default +installation's approvals. The same boundary applies to legacy +`plugin-binding-approvals.json` imports into shared SQLite state. Example schema: diff --git a/scripts/e2e/lib/doctor-install-switch/scenario.sh b/scripts/e2e/lib/doctor-install-switch/scenario.sh index 55073c62a60f..b9bacacb1ba4 100644 --- a/scripts/e2e/lib/doctor-install-switch/scenario.sh +++ b/scripts/e2e/lib/doctor-install-switch/scenario.sh @@ -55,6 +55,7 @@ git_cli="$git_root/openclaw.mjs" package_version="$(node -p "require(\"$npm_root/package.json\").version")" update_doctor_env="OPENCLAW_UPDATE_IN_PROGRESS=1" +update_doctor_env+=" OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS=1" update_doctor_env+=" OPENCLAW_UPDATE_PARENT_SUPPORTS_DOCTOR_CONFIG_WRITE=1" update_doctor_env+=" OPENCLAW_UPDATE_PARENT_SUPPORTS_GATEWAY_RESTART=1" update_doctor_env+=" OPENCLAW_UPDATE_PARENT_ALLOWS_GATEWAY_SERVICE_REPAIR=1" @@ -161,6 +162,7 @@ run_flow() { fi assert_entrypoint "$unit_path" "$doctor_expected" + assert_no_env_key "$unit_path" "OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS" } run_flow \ @@ -177,6 +179,85 @@ run_flow \ "$update_doctor_env $npm_bin doctor --repair --force --yes --non-interactive" \ "$npm_entry" +plugin_binding_approval_count() { + local database_path="$1" + if [ ! -f "$database_path" ]; then + echo "0" + return + fi + node --no-warnings - "$database_path" <<'NODE' +const { DatabaseSync } = require("node:sqlite"); +const database = new DatabaseSync(process.argv[2]); +const table = database + .prepare("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = ?") + .get("plugin_binding_approvals"); +const row = table + ? database.prepare("SELECT COUNT(*) AS count FROM plugin_binding_approvals").get() + : { count: 0 }; +database.close(); +process.stdout.write(String(row.count)); +NODE +} + +run_cross_state_approval_flow() { + local name="cross-state-approvals" + local automated_log="/tmp/openclaw-doctor-switch-${name}-automated.log" + local direct_log="/tmp/openclaw-doctor-switch-${name}-direct.log" + local command_timeout="${OPENCLAW_DOCKER_DOCTOR_SWITCH_COMMAND_TIMEOUT:-900s}" + + echo "== Flow: $name ==" + openclaw_test_state_create "switch-${name}" empty + export USER="testuser" + + local default_state_dir="$HOME/.openclaw" + local custom_state_dir="$HOME/custom-state" + local exec_source="$default_state_dir/exec-approvals.json" + local plugin_source="$default_state_dir/plugin-binding-approvals.json" + local state_database="$custom_state_dir/state/openclaw.sqlite" + mkdir -p "$default_state_dir" "$custom_state_dir" + printf '%s\n' '{"version":1,"socket":{"token":"legacy-token"},"defaults":{"security":"deny","ask":"always"}}' >"$exec_source" + printf '%s\n' '{"version":1,"approvals":[{"pluginRoot":"/plugins/codex-a","pluginId":"codex","channel":"telegram","accountId":"default","approvedAt":2345}]}' >"$plugin_source" + local exec_source_hash + local plugin_source_hash + exec_source_hash="$(sha256sum "$exec_source" | awk '{print $1}')" + plugin_source_hash="$(sha256sum "$plugin_source" | awk '{print $1}')" + + if ! openclaw_e2e_maybe_timeout "$command_timeout" env \ + OPENCLAW_STATE_DIR="$custom_state_dir" \ + OPENCLAW_CONFIG_PATH="$custom_state_dir/openclaw.json" \ + OPENCLAW_UPDATE_IN_PROGRESS=1 \ + "$npm_bin" doctor --repair --yes --non-interactive >"$automated_log" 2>&1; then + openclaw_e2e_print_log "$automated_log" + exit 1 + fi + + test "$(sha256sum "$exec_source" | awk '{print $1}')" = "$exec_source_hash" + test "$(sha256sum "$plugin_source" | awk '{print $1}')" = "$plugin_source_hash" + test ! -e "$exec_source.migrated" + test ! -e "$plugin_source.migrated" + test ! -e "$custom_state_dir/exec-approvals.json" + test "$(plugin_binding_approval_count "$state_database")" = "0" + + if ! openclaw_e2e_maybe_timeout "$command_timeout" env \ + -u OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS \ + -u OPENCLAW_UPDATE_IN_PROGRESS \ + OPENCLAW_STATE_DIR="$custom_state_dir" \ + OPENCLAW_CONFIG_PATH="$custom_state_dir/openclaw.json" \ + "$npm_bin" doctor --repair --yes --non-interactive >"$direct_log" 2>&1; then + openclaw_e2e_print_log "$direct_log" + exit 1 + fi + + test ! -e "$exec_source" + test ! -e "$plugin_source" + test "$(sha256sum "$exec_source.migrated" | awk '{print $1}')" = "$exec_source_hash" + test "$(sha256sum "$plugin_source.migrated" | awk '{print $1}')" = "$plugin_source_hash" + test -e "$custom_state_dir/exec-approvals.json" + test "$(plugin_binding_approval_count "$state_database")" = "1" +} + +run_cross_state_approval_flow + run_proxy_env_flow() { local name="proxy-env-cleanup" local install_log="/tmp/openclaw-doctor-switch-${name}-install.log" @@ -206,6 +287,7 @@ run_proxy_env_flow() { } >>"$unit_path" if ! openclaw_e2e_maybe_timeout "$command_timeout" env \ OPENCLAW_UPDATE_IN_PROGRESS=1 \ + OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS=1 \ OPENCLAW_UPDATE_PARENT_SUPPORTS_DOCTOR_CONFIG_WRITE=1 \ OPENCLAW_UPDATE_PARENT_SUPPORTS_GATEWAY_RESTART=1 \ OPENCLAW_UPDATE_PARENT_ALLOWS_GATEWAY_SERVICE_REPAIR=1 \ @@ -216,6 +298,7 @@ run_proxy_env_flow() { fi assert_no_env_key "$unit_path" "HTTP_PROXY" assert_no_env_key "$unit_path" "HTTPS_PROXY" + assert_no_env_key "$unit_path" "OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS" } run_proxy_env_flow diff --git a/scripts/watch-node.mjs b/scripts/watch-node.mjs index f92086ec0fdd..9235283888e0 100644 --- a/scripts/watch-node.mjs +++ b/scripts/watch-node.mjs @@ -20,6 +20,10 @@ const WATCH_LOCK_POLL_MS = 100; const WATCH_SHUTDOWN_KILL_GRACE_MS = 5_000; const WATCH_LOCK_DIR = path.join(".local", "watch-node"); const AUTO_DOCTOR_DISABLE_VALUES = new Set(["0", "false", "no", "off"]); +// The source watcher cannot import the TypeScript owner; keep this literal +// aligned with src/commands/doctor-invocation.ts. +const DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV = + "OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS"; const buildRunnerArgs = (args) => [WATCH_NODE_RUNNER, ...args]; const buildDoctorRunnerArgs = () => [WATCH_NODE_RUNNER, "doctor", "--fix", "--non-interactive"]; @@ -452,7 +456,10 @@ export async function runWatchMain(params = {}) { watchProcess = deps.spawn(deps.process.execPath, buildDoctorRunnerArgs(), { cwd: deps.cwd, detached: useChildProcessGroup, - env: childEnv, + env: { + ...childEnv, + [DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV]: "1", + }, stdio: "inherit", }); watchProcess.on("error", (error) => { diff --git a/src/cli/program/config-guard.test.ts b/src/cli/program/config-guard.test.ts index 7de719ab5db0..6fe558955828 100644 --- a/src/cli/program/config-guard.test.ts +++ b/src/cli/program/config-guard.test.ts @@ -182,6 +182,7 @@ describe("ensureConfigReady", () => { migrateState: true, migrateLegacyConfig: false, invalidConfigNote: false, + crossStateDirImports: false, }); } }); @@ -203,6 +204,7 @@ describe("ensureConfigReady", () => { migrateLegacyConfig: false, invalidConfigNote: false, observe: false, + crossStateDirImports: false, }); }); @@ -217,6 +219,7 @@ describe("ensureConfigReady", () => { migrateLegacyConfig: false, invalidConfigNote: false, observe: false, + crossStateDirImports: false, }); }); @@ -227,6 +230,7 @@ describe("ensureConfigReady", () => { migrateState: true, migrateLegacyConfig: false, invalidConfigNote: false, + crossStateDirImports: false, requireStartupMigrationCheckpoint: true, }); }); @@ -259,6 +263,7 @@ describe("ensureConfigReady", () => { migrateState: true, migrateLegacyConfig: false, invalidConfigNote: false, + crossStateDirImports: false, }); }); @@ -282,6 +287,7 @@ describe("ensureConfigReady", () => { migrateState: true, migrateLegacyConfig: false, invalidConfigNote: false, + crossStateDirImports: false, }); }); @@ -306,6 +312,7 @@ describe("ensureConfigReady", () => { migrateState: true, migrateLegacyConfig: false, invalidConfigNote: false, + crossStateDirImports: false, }); expect(setRuntimeConfigSnapshotMock).toHaveBeenCalledWith( migratedSnapshot.runtimeConfig, @@ -322,18 +329,36 @@ describe("ensureConfigReady", () => { expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledOnce(); }); - it("does not run doctor flow for default-state-dir exec approvals when a custom state dir is set", async () => { - // Cross-state-dir imports are doctor-owned; the implicit preflight must not - // trigger (and must never archive) files that belong to the default dir. - const root = useTempOpenClawHome(); - const stateDir = path.join(root, "custom-state"); - setTestEnvValue("OPENCLAW_STATE_DIR", stateDir); - writeStateMarker(root, "exec-approvals.json"); + it.each([ + { commandPath: ["agent"], source: "exec-approvals.json" }, + { commandPath: ["status"], source: "plugin-binding-approvals.json" }, + { commandPath: ["plugins", "list"], source: "exec-approvals.json" }, + { commandPath: ["tasks", "list"], source: "plugin-binding-approvals.json" }, + ])( + "runs notice-only preflight for $commandPath with default-state $source", + async ({ commandPath, source }) => { + const root = useTempOpenClawHome(); + const stateDir = path.join(root, "custom-state"); + setTestEnvValue("OPENCLAW_STATE_DIR", stateDir); + writeStateMarker(root, source); + const sourcePath = path.join(root, ".openclaw", source); + const sourceRaw = fs.readFileSync(sourcePath, "utf8"); - await runEnsureConfigReady(["agent"]); + await runEnsureConfigReady(commandPath); - expect(loadAndMaybeMigrateDoctorConfigMock).not.toHaveBeenCalled(); - }); + expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledOnce(); + expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledWith({ + migrateState: true, + migrateLegacyConfig: false, + invalidConfigNote: false, + ...(commandPath[0] === "status" ? { observe: false } : {}), + crossStateDirImports: false, + }); + expect(fs.readFileSync(sourcePath, "utf8")).toBe(sourceRaw); + expect(fs.existsSync(`${sourcePath}.migrated`)).toBe(false); + expect(fs.existsSync(path.join(stateDir, "exec-approvals.json"))).toBe(false); + }, + ); it.each([ ["Discord model picker preferences", "discord/model-picker-preferences.json"], diff --git a/src/cli/program/config-guard.ts b/src/cli/program/config-guard.ts index 9ab6b9b9154e..6eda8edc4045 100644 --- a/src/cli/program/config-guard.ts +++ b/src/cli/program/config-guard.ts @@ -4,7 +4,12 @@ import os from "node:os"; import path from "node:path"; import { withSuppressedNotes } from "../../../packages/terminal-core/src/note.js"; import { readConfigFileSnapshot, setRuntimeConfigSnapshot } from "../../config/config.js"; -import { resolveLegacyStateDirs, resolveOAuthDir, resolveStateDir } from "../../config/paths.js"; +import { + resolveLegacyStateDirs, + resolveNewStateDir, + resolveOAuthDir, + resolveStateDir, +} from "../../config/paths.js"; import type { ConfigFileSnapshot } from "../../config/types.js"; import { resolveRequiredHomeDir } from "../../infra/home-dir.js"; import { ExitError, type RuntimeEnv } from "../../runtime.js"; @@ -98,6 +103,23 @@ function hasBundledChannelLegacyStateMigrationInputs(stateDir: string, oauthDir: return dirHasFile(oauthDir, isLegacyWhatsAppAuthFile); } +function hasCrossStateDirApprovalMigrationInputs(stateDir: string): boolean { + if (!process.env.OPENCLAW_STATE_DIR?.trim()) { + return false; + } + const homeDir = resolveRequiredHomeDir(process.env, os.homedir); + const defaultStateDir = resolveNewStateDir(() => homeDir); + if (path.resolve(defaultStateDir) === path.resolve(stateDir)) { + return false; + } + const execApprovalsSource = path.join(defaultStateDir, "exec-approvals.json"); + const execApprovalsTarget = path.join(stateDir, "exec-approvals.json"); + return ( + (fileOrDirExists(execApprovalsSource) && !fileOrDirExists(execApprovalsTarget)) || + fileOrDirExists(path.join(defaultStateDir, "plugin-binding-approvals.json")) + ); +} + function hasPendingSqliteSidecarArchive(sourcePath: string): boolean { return ( fileOrDirExists(`${sourcePath}.migrated`) && @@ -133,7 +155,8 @@ function hasLegacyStateMigrationInputs(): boolean { sqliteSidecarPaths.some( (sourcePath) => fileOrDirExists(sourcePath) || hasPendingSqliteSidecarArchive(sourcePath), ) || - hasBundledChannelLegacyStateMigrationInputs(stateDir, oauthDir) + hasBundledChannelLegacyStateMigrationInputs(stateDir, oauthDir) || + hasCrossStateDirApprovalMigrationInputs(stateDir) ); } @@ -209,6 +232,7 @@ export async function ensureConfigReady(params: { migrateLegacyConfig: false, invalidConfigNote: false, ...(commandName === "status" ? { observe: false } : {}), + crossStateDirImports: false, ...(shouldRequireStartupMigrationCheckpoint(commandPath) ? { requireStartupMigrationCheckpoint: true } : {}), diff --git a/src/cli/program/register.maintenance.test.ts b/src/cli/program/register.maintenance.test.ts index e905a3c2391a..cd226ab410ff 100644 --- a/src/cli/program/register.maintenance.test.ts +++ b/src/cli/program/register.maintenance.test.ts @@ -1,6 +1,7 @@ // Register maintenance tests cover maintenance command registration in the CLI program. import { Command } from "commander"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV } from "../../commands/doctor-invocation.js"; import { registerMaintenanceCommands } from "./register.maintenance.js"; const mocks = vi.hoisted(() => ({ @@ -68,6 +69,10 @@ describe("registerMaintenanceCommands doctor action", () => { vi.clearAllMocks(); }); + afterEach(() => { + vi.unstubAllEnvs(); + }); + it("exits with code 0 after successful doctor run", async () => { doctorCommand.mockResolvedValue(undefined); @@ -101,6 +106,28 @@ describe("registerMaintenanceCommands doctor action", () => { const [runtimeArg, options] = commandCall(doctorCommand); expect(runtimeArg).toBe(runtime); expect(options.repair).toBe(true); + expect(options.crossStateDirImports).toBe(true); + }); + + it("denies cross-state imports when an automation parent disables them", async () => { + doctorCommand.mockResolvedValue(undefined); + vi.stubEnv(DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV, "1"); + + await runMaintenanceCli(["doctor", "--fix", "--non-interactive"]); + + const [, options] = commandCall(doctorCommand); + expect(options.repair).toBe(true); + expect(options.crossStateDirImports).toBe(false); + }); + + it("denies cross-state imports for older update parents", async () => { + doctorCommand.mockResolvedValue(undefined); + vi.stubEnv("OPENCLAW_UPDATE_IN_PROGRESS", "1"); + + await runMaintenanceCli(["doctor", "--fix", "--non-interactive"]); + + const [, options] = commandCall(doctorCommand); + expect(options.crossStateDirImports).toBe(false); }); it("runs doctor lint mode without invoking repair doctor", async () => { diff --git a/src/cli/program/register.maintenance.ts b/src/cli/program/register.maintenance.ts index 83090498c2ee..3a8ad1b63153 100644 --- a/src/cli/program/register.maintenance.ts +++ b/src/cli/program/register.maintenance.ts @@ -2,6 +2,7 @@ import type { Command } from "commander"; import { formatDocsLink } from "../../../packages/terminal-core/src/links.js"; import { theme } from "../../../packages/terminal-core/src/theme.js"; +import { resolveDoctorCrossStateDirImports } from "../../commands/doctor-invocation.js"; import { defaultRuntime } from "../../runtime.js"; import { runCommandWithRuntime } from "../cli-utils.js"; @@ -96,6 +97,7 @@ export function registerMaintenanceCommands(program: Command) { deep: Boolean(opts.deep), postUpgrade: Boolean(opts.postUpgrade), json: Boolean(opts.json), + crossStateDirImports: resolveDoctorCrossStateDirImports(), }); defaultRuntime.exit(0); }); diff --git a/src/cli/update-cli.test.ts b/src/cli/update-cli.test.ts index 02d849147116..aac8f7d41000 100644 --- a/src/cli/update-cli.test.ts +++ b/src/cli/update-cli.test.ts @@ -7406,6 +7406,7 @@ describe("update-cli", () => { nonInteractive: true, repair: true, yes: true, + crossStateDirImports: false, }); expect(syncPluginCall()?.channel).toBe("stable"); expect(syncPluginCall()?.acknowledgeClawHubRisk).toBe(true); @@ -7505,6 +7506,7 @@ describe("update-cli", () => { nonInteractive: true, repair: true, yes: false, + crossStateDirImports: false, }); expect(syncPluginCall()?.channel).toBe("beta"); expect(syncPluginCall()?.config).toEqual({ diff --git a/src/cli/update-cli/update-command.test.ts b/src/cli/update-cli/update-command.test.ts index 5568535bd784..6d58a71f7435 100644 --- a/src/cli/update-cli/update-command.test.ts +++ b/src/cli/update-cli/update-command.test.ts @@ -3,6 +3,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it, vi } from "vitest"; +import { DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV } from "../../commands/doctor-invocation.js"; import { buildGatewayInstallEntrypointCandidates as resolveGatewayInstallEntrypointCandidates, resolveGatewayInstallEntrypoint, @@ -216,6 +217,7 @@ describe("resolvePostInstallDoctorEnv", () => { expect(env.PATH).toBe("/bin"); expect(env.NODE_DISABLE_COMPILE_CACHE).toBe("1"); + expect(env[DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV]).toBe("1"); expect(env.OPENCLAW_STATE_DIR).toBe(path.join("/srv/openclaw", "daemon-state")); expect(env.OPENCLAW_CONFIG_PATH).toBe( path.join("/srv/openclaw", "daemon-state", "openclaw.json"), @@ -234,6 +236,7 @@ describe("resolvePostInstallDoctorEnv", () => { expect(env.PATH).toBe("/bin"); expect(env.NODE_DISABLE_COMPILE_CACHE).toBe("1"); + expect(env[DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV]).toBe("1"); expect(env.OPENCLAW_STATE_DIR).toBe("/caller/state"); expect(env.OPENCLAW_PROFILE).toBe("caller"); }); diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index 8eed7665a256..2abd2aa288f2 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -16,6 +16,7 @@ import { checkShellCompletionStatus, ensureCompletionCacheExists, } from "../../commands/doctor-completion.js"; +import { DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV } from "../../commands/doctor-invocation.js"; import { doctorCommand } from "../../commands/doctor.js"; import { UPDATE_DEFER_CONFIGURED_PLUGIN_INSTALL_REPAIR_ENV, @@ -1494,7 +1495,10 @@ export function resolvePostInstallDoctorEnv(params?: { serviceEnv?: NodeJS.ProcessEnv; invocationCwd?: string; }): NodeJS.ProcessEnv { - const resolvedEnv = disableUpdatedPackageCompileCacheEnv(params?.baseEnv ?? process.env); + const resolvedEnv: NodeJS.ProcessEnv = { + ...disableUpdatedPackageCompileCacheEnv(params?.baseEnv ?? process.env), + [DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV]: "1", + }; if (!params?.serviceEnv) { return resolvedEnv; } @@ -2802,6 +2806,7 @@ async function maybeRestartService(params: { process.stdin.isTTY && !params.opts.json && params.opts.yes !== true; await doctorCommand(defaultRuntime, { nonInteractive: !interactiveDoctor, + crossStateDirImports: false, }); } catch (err) { defaultRuntime.log(theme.warn(`Doctor failed: ${String(err)}`)); @@ -2988,6 +2993,7 @@ export async function updateFinalizeCommand(opts: UpdateFinalizeOptions): Promis nonInteractive: true, repair: true, yes: opts.yes === true, + crossStateDirImports: false, }); configSnapshot = await readConfigFileSnapshot({ skipPluginValidation: true }); if (requestedChannel) { @@ -3478,6 +3484,7 @@ async function continuePostCoreUpdateInFreshProcess(params: { env: { ...stripGatewayServiceMarkerEnv(disableUpdatedPackageCompileCacheEnv(process.env)), OPENCLAW_UPDATE_IN_PROGRESS: "1", + [DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV]: "1", [POST_CORE_UPDATE_ENV]: "1", [POST_CORE_UPDATE_CHANNEL_ENV]: params.channel, ...(params.requestedChannel diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index c8e47406583c..809e2b99d396 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -14,6 +14,7 @@ type TerminalNote = (message: string, title?: string) => void; const terminalNoteMock = vi.hoisted(() => vi.fn()); const callGatewayMock = vi.hoisted(() => vi.fn()); const runDoctorRepairSequenceMock = vi.hoisted(() => vi.fn()); +const runDoctorConfigPreflightOptionsMock = vi.hoisted(() => vi.fn()); const collectDoctorPreviewNotesParamsMock = vi.hoisted(() => vi.fn()); const collectImplicitFallbackClobberWarningsMock = vi.hoisted(() => vi.fn<(cfg: unknown) => string[]>(() => []), @@ -1301,7 +1302,8 @@ vi.mock("./doctor-config-preflight.js", async () => { } return { - runDoctorConfigPreflight: vi.fn(async () => { + runDoctorConfigPreflight: vi.fn(async (options: unknown) => { + runDoctorConfigPreflightOptionsMock(options); const injected = getDoctorConfigInputForTest(); const configPath = injected?.path ?? resolveConfigPath(); let parsed: Record = injected?.config @@ -1530,6 +1532,31 @@ describe("doctor config flow", () => { collectImplicitFallbackClobberWarningsMock.mockClear(); collectImplicitFallbackClobberWarningsMock.mockReturnValue([]); noteImplicitFallbackClobberWarningsMock.mockClear(); + runDoctorConfigPreflightOptionsMock.mockClear(); + }); + + it("grants config preflight cross-state imports only with repair and direct capability", async () => { + await runDoctorConfigWithInput({ + config: {}, + repair: true, + run: ({ options, confirm }) => + loadAndMaybeMigrateDoctorConfig({ + options: { ...options, crossStateDirImports: true }, + confirm: async () => confirm(), + }), + }); + expect(runDoctorConfigPreflightOptionsMock).toHaveBeenLastCalledWith( + expect.objectContaining({ crossStateDirImports: true }), + ); + + await runDoctorConfigWithInput({ + config: {}, + repair: true, + run: loadAndMaybeMigrateDoctorConfig, + }); + expect(runDoctorConfigPreflightOptionsMock).toHaveBeenLastCalledWith( + expect.objectContaining({ crossStateDirImports: false }), + ); }); it("preserves invalid config for doctor repairs", async () => { diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 0e8ab788aeb0..483e64721a3f 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -141,7 +141,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { const preflight = await runDoctorConfigPreflight({ repairPrefixedConfig: shouldRepair, recoverCorruptTargetStore: shouldRepair, - crossStateDirImports: shouldRepair, + crossStateDirImports: shouldRepair && params.options.crossStateDirImports === true, }); const snapshot = preflight.snapshot; const baseCfg = preflight.baseConfig; diff --git a/src/commands/doctor-invocation.ts b/src/commands/doctor-invocation.ts new file mode 100644 index 000000000000..ed936152e227 --- /dev/null +++ b/src/commands/doctor-invocation.ts @@ -0,0 +1,16 @@ +/** Internal doctor invocation capabilities shared by direct and automated callers. */ +import { isTruthyEnvValue } from "../infra/env.js"; +import { UPDATE_IN_PROGRESS_ENV } from "./doctor/shared/update-phase.js"; + +export const DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV = + "OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS"; + +/** Direct CLI doctor owns cross-state imports unless its automation parent denies them. */ +export function resolveDoctorCrossStateDirImports(env: NodeJS.ProcessEnv = process.env): boolean { + // Older update parents know only OPENCLAW_UPDATE_IN_PROGRESS. Treat that + // existing cross-version handshake as deny-by-default for a newer doctor. + return !( + isTruthyEnvValue(env[DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV]) || + isTruthyEnvValue(env[UPDATE_IN_PROGRESS_ENV]) + ); +} diff --git a/src/commands/doctor.types.ts b/src/commands/doctor.types.ts index 3b55076e91ca..79ca4cd4dd03 100644 --- a/src/commands/doctor.types.ts +++ b/src/commands/doctor.types.ts @@ -10,4 +10,6 @@ export type DoctorOptions = { allowExec?: boolean; postUpgrade?: boolean; json?: boolean; + /** Internal capability granted only to direct operator-owned doctor invocations. */ + crossStateDirImports?: boolean; }; diff --git a/src/flows/doctor-health-contributions.test.ts b/src/flows/doctor-health-contributions.test.ts index 4fbc13eaa836..114baff0bcb8 100644 --- a/src/flows/doctor-health-contributions.test.ts +++ b/src/flows/doctor-health-contributions.test.ts @@ -1446,6 +1446,10 @@ describe("doctor health contributions", () => { await contribution.run(ctx); + expect(mocks.detectLegacyStateMigrations).toHaveBeenCalledWith({ + cfg, + crossStateDirImports: false, + }); expect(mocks.runLegacyStateMigrations).toHaveBeenCalledWith({ detected, config: cfg, @@ -1453,6 +1457,48 @@ describe("doctor health contributions", () => { }); }); + it("grants legacy-state cross-state imports only to capable doctor origins", async () => { + const contribution = requireDoctorContribution("doctor:legacy-state"); + const detected = { preview: [], warnings: [], notices: [] }; + mocks.detectLegacyStateMigrations.mockResolvedValue(detected); + + const directRepairContext = { + cfg: {}, + sourceConfigValid: true, + prompter: buildDoctorPrompter(true), + runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() }, + options: { nonInteractive: true, repair: true, crossStateDirImports: true }, + } as unknown as Parameters<(typeof contribution)["run"]>[0]; + await contribution.run(directRepairContext); + expect(mocks.detectLegacyStateMigrations).toHaveBeenLastCalledWith({ + cfg: {}, + crossStateDirImports: true, + }); + + const interactivePrompter = buildDoctorPrompter(false); + interactivePrompter.repairMode.canPrompt = true; + interactivePrompter.repairMode.nonInteractive = false; + await contribution.run({ + ...directRepairContext, + prompter: interactivePrompter, + options: { crossStateDirImports: true }, + }); + expect(mocks.detectLegacyStateMigrations).toHaveBeenLastCalledWith({ + cfg: {}, + crossStateDirImports: true, + }); + + const automatedRepairContext = { + ...directRepairContext, + options: { nonInteractive: true, repair: true, crossStateDirImports: false }, + }; + await contribution.run(automatedRepairContext); + expect(mocks.detectLegacyStateMigrations).toHaveBeenLastCalledWith({ + cfg: {}, + crossStateDirImports: false, + }); + }); + it("prints legacy state migration notices during manual doctor runs", async () => { const contribution = requireDoctorContribution("doctor:legacy-state"); const detected = { preview: ["legacy sessions"], warnings: [], notices: [] }; diff --git a/src/flows/doctor-health-contributions.ts b/src/flows/doctor-health-contributions.ts index 320131d33704..7c27d8ff5b2d 100644 --- a/src/flows/doctor-health-contributions.ts +++ b/src/flows/doctor-health-contributions.ts @@ -518,16 +518,15 @@ async function runLegacyStateHealth(ctx: DoctorHealthFlowContext): Promise const { detectLegacyStateMigrations, runLegacyStateMigrations } = await import("../commands/doctor-state-migrations.js"); const { note } = await loadNoteModule(); - // Cross-state-dir imports (default home dir -> OPENCLAW_STATE_DIR) are - // allowed here only when the operator either confirms the previewed plan - // interactively or asked for repair; a bare non-interactive doctor stays - // read-only toward the default state dir. + // Only a direct operator-owned doctor may inspect the default state dir for + // imports. Automated repair callers explicitly lack this capability so a + // temporary OPENCLAW_STATE_DIR cannot capture and archive production trust. + const operatorCanApproveCrossStateDirImports = + ctx.prompter.repairMode.canPrompt || ctx.prompter.shouldRepair; const legacyState = await detectLegacyStateMigrations({ cfg: ctx.cfg, crossStateDirImports: - ctx.options.nonInteractive !== true || - ctx.options.repair === true || - ctx.options.yes === true, + ctx.options.crossStateDirImports === true && operatorCanApproveCrossStateDirImports, }); if (legacyState.warnings.length > 0) { note(legacyState.warnings.join("\n"), "Doctor warnings"); diff --git a/src/infra/update-runner.test.ts b/src/infra/update-runner.test.ts index d9c0d2a4c43a..9fd39cde5f3d 100644 --- a/src/infra/update-runner.test.ts +++ b/src/infra/update-runner.test.ts @@ -960,6 +960,7 @@ describe("runGatewayUpdate", () => { expect(result.status).toBe("ok"); expect(doctorEnv?.OPENCLAW_UPDATE_IN_PROGRESS).toBe("1"); + expect(doctorEnv?.OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS).toBe("1"); expect(doctorEnv?.OPENCLAW_UPDATE_DEFER_CONFIGURED_PLUGIN_INSTALL_REPAIR).toBe("1"); expect(doctorEnv?.OPENCLAW_UPDATE_PARENT_SUPPORTS_DOCTOR_CONFIG_WRITE).toBe("1"); expect(doctorEnv?.OPENCLAW_UPDATE_PARENT_SUPPORTS_GATEWAY_RESTART).toBe("1"); @@ -2688,6 +2689,7 @@ describe("runGatewayUpdate", () => { expect(calls).toContain(doctorCommand); expect(result.steps.map((step) => step.name)).toContain("openclaw doctor"); expect(doctorEnv?.OPENCLAW_UPDATE_IN_PROGRESS).toBe("1"); + expect(doctorEnv?.OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS).toBe("1"); expect(doctorEnv?.OPENCLAW_UPDATE_PARENT_SUPPORTS_DOCTOR_CONFIG_WRITE).toBe("1"); expect(doctorEnv?.OPENCLAW_UPDATE_PARENT_SUPPORTS_GATEWAY_RESTART).toBe("1"); expect(doctorEnv?.OPENCLAW_UPDATE_PARENT_ALLOWS_GATEWAY_SERVICE_REPAIR).toBe("1"); diff --git a/src/infra/update-runner.ts b/src/infra/update-runner.ts index f34463775efb..5aede59dd0ed 100644 --- a/src/infra/update-runner.ts +++ b/src/infra/update-runner.ts @@ -6,6 +6,7 @@ import { normalizeStringEntries, uniqueStrings, } from "@openclaw/normalization-core/string-normalization"; +import { DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV } from "../commands/doctor-invocation.js"; import { resolveGatewayInstallEntrypoint } from "../daemon/gateway-entrypoint.js"; import { type CommandOptions, runCommandWithTimeout } from "../process/exec.js"; import { @@ -1628,6 +1629,7 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise< const doctorStep = await runStep( step("openclaw doctor", doctorArgv, gitRoot, { OPENCLAW_UPDATE_IN_PROGRESS: "1", + [DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV]: "1", ...(opts.deferConfiguredPluginInstallRepair ? { [UPDATE_DEFER_CONFIGURED_PLUGIN_INSTALL_REPAIR_ENV]: "1" } : {}), @@ -1823,6 +1825,7 @@ export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise< timeoutMs, env: { OPENCLAW_UPDATE_IN_PROGRESS: "1", + [DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS_ENV]: "1", [UPDATE_PARENT_SUPPORTS_DOCTOR_CONFIG_WRITE_ENV]: "1", [UPDATE_PARENT_SUPPORTS_GATEWAY_RESTART_ENV]: "1", [UPDATE_PARENT_ALLOWS_GATEWAY_SERVICE_REPAIR_ENV]: allowGatewayServiceRepair diff --git a/src/infra/watch-node.test.ts b/src/infra/watch-node.test.ts index d572a58f4c43..04ee6f29e342 100644 --- a/src/infra/watch-node.test.ts +++ b/src/infra/watch-node.test.ts @@ -343,7 +343,7 @@ describe("watch-node script", () => { .mockReturnValueOnce(gatewayA) .mockReturnValueOnce(doctor) .mockReturnValueOnce(gatewayB); - const { watcher, fakeProcess, runPromise } = startWatchRun({ spawn }); + const { watcher, fakeProcess, runPromise } = startWatchRun({ env: {}, spawn }); gatewayA.emit("exit", 1, null); await new Promise((resolve) => { @@ -360,6 +360,7 @@ describe("watch-node script", () => { "--non-interactive", ]); expect(requireSpawnOptions(spawn, 1).stdio).toBe("inherit"); + expect(requireSpawnEnv(spawn, 1).OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS).toBe("1"); doctor.emit("exit", 0, null); await new Promise((resolve) => { @@ -371,6 +372,9 @@ describe("watch-node script", () => { expect(restartedGatewaySpawnCall[0]).toBe("/usr/local/bin/node"); expect(restartedGatewaySpawnCall[1]).toEqual(["scripts/run-node.mjs", "gateway", "--force"]); expect(requireSpawnOptions(spawn, 2).stdio).toBe("inherit"); + expect( + requireSpawnEnv(spawn, 2).OPENCLAW_DOCTOR_DISABLE_CROSS_STATE_DIR_IMPORTS, + ).toBeUndefined(); fakeProcess.emit("SIGINT"); const exitCode = await runPromise;