fix(doctor): gate cross-state-dir legacy imports behind explicit doctor opt-in (#103247)

* fix(doctor): gate cross-state-dir legacy imports behind explicit doctor opt-in

A gateway or CLI command started with OPENCLAW_STATE_DIR pointing at a
non-default directory used to import legacy files FROM the default
~/.openclaw state dir (exec-approvals.json, plugin-binding-approvals.json)
and archive the originals with a .migrated suffix. Any isolated, test, or
staging run on a host with production state silently captured and archived
the production files.

Cross-state-dir imports now run only with the new crossStateDirImports
opt-in: openclaw doctor --fix (or an interactive doctor confirm) performs
the import; the implicit CLI/gateway preflight leaves the default dir
untouched and emits a notice pointing at doctor instead.

* test(doctor): include notices in legacy-state detector mocks
This commit is contained in:
Peter Steinberger
2026-07-10 03:56:02 +01:00
committed by GitHub
parent 0fa49ad4dd
commit 2b3dc3042f
11 changed files with 243 additions and 43 deletions
+7
View File
@@ -89,6 +89,13 @@ 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
`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.
Example schema:
```json
+4 -7
View File
@@ -314,7 +314,9 @@ describe("ensureConfigReady", () => {
expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledOnce();
});
it("runs doctor flow before agent commands when default exec approvals must move to a custom state dir", async () => {
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);
@@ -322,12 +324,7 @@ describe("ensureConfigReady", () => {
await runEnsureConfigReady(["agent"]);
expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledOnce();
expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledWith({
migrateState: true,
migrateLegacyConfig: false,
invalidConfigNote: false,
});
expect(loadAndMaybeMigrateDoctorConfigMock).not.toHaveBeenCalled();
});
it.each([
+1 -16
View File
@@ -98,20 +98,6 @@ function hasBundledChannelLegacyStateMigrationInputs(stateDir: string, oauthDir:
return dirHasFile(oauthDir, isLegacyWhatsAppAuthFile);
}
function hasLegacyExecApprovalsMigrationInput(stateDir: string): boolean {
if (!process.env.OPENCLAW_STATE_DIR?.trim()) {
return false;
}
const homeDir = resolveRequiredHomeDir(process.env, os.homedir);
const sourcePath = path.join(homeDir, ".openclaw", "exec-approvals.json");
const targetPath = path.join(stateDir, "exec-approvals.json");
return (
path.resolve(sourcePath) !== path.resolve(targetPath) &&
fileOrDirExists(sourcePath) &&
!fileOrDirExists(targetPath)
);
}
function hasPendingSqliteSidecarArchive(sourcePath: string): boolean {
return (
fileOrDirExists(`${sourcePath}.migrated`) &&
@@ -147,8 +133,7 @@ function hasLegacyStateMigrationInputs(): boolean {
sqliteSidecarPaths.some(
(sourcePath) => fileOrDirExists(sourcePath) || hasPendingSqliteSidecarArchive(sourcePath),
) ||
hasBundledChannelLegacyStateMigrationInputs(stateDir, oauthDir) ||
hasLegacyExecApprovalsMigrationInput(stateDir)
hasBundledChannelLegacyStateMigrationInputs(stateDir, oauthDir)
);
}
+1
View File
@@ -141,6 +141,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
const preflight = await runDoctorConfigPreflight({
repairPrefixedConfig: shouldRepair,
recoverCorruptTargetStore: shouldRepair,
crossStateDirImports: shouldRepair,
});
const snapshot = preflight.snapshot;
const baseCfg = preflight.baseConfig;
+16 -2
View File
@@ -181,6 +181,13 @@ export async function runDoctorConfigPreflight(
/** Return false or reject on config drift; the preflight always unwinds owned resources. */
beforeStateMigrations?: (snapshot?: ConfigFileSnapshot) => Promise<boolean>;
requireStartupMigrationCheckpoint?: boolean;
/**
* Allows legacy imports whose source lives in the DEFAULT home state dir
* while OPENCLAW_STATE_DIR points elsewhere. Only explicit doctor repair
* runs opt in; the implicit CLI/gateway preflight must never archive
* files that belong to another install's state dir.
*/
crossStateDirImports?: boolean;
} = {},
): Promise<DoctorConfigPreflightResult> {
const stateMigrations =
@@ -315,6 +322,7 @@ export async function runDoctorConfigPreflight(
: {}),
env: process.env,
recoverCorruptTargetStore: options.recoverCorruptTargetStore,
crossStateDirImports: options.crossStateDirImports,
}),
);
} else if (stateMigrationInput.pluginDoctorConfig) {
@@ -325,12 +333,18 @@ export async function runDoctorConfigPreflight(
}),
);
noteStartupStateMigrationResult(
await autoMigrateLegacyTaskStateSidecars({ env: process.env }),
await autoMigrateLegacyTaskStateSidecars({
env: process.env,
crossStateDirImports: options.crossStateDirImports,
}),
);
}
} else {
noteStartupStateMigrationResult(
await autoMigrateLegacyTaskStateSidecars({ env: process.env }),
await autoMigrateLegacyTaskStateSidecars({
env: process.env,
crossStateDirImports: options.crossStateDirImports,
}),
);
}
}
+72 -1
View File
@@ -2416,6 +2416,7 @@ describe("doctor legacy state migrations", () => {
cfg: {},
env: { OPENCLAW_STATE_DIR: stateDir } as NodeJS.ProcessEnv,
homedir: () => root,
crossStateDirImports: true,
});
expect(detected.preview).toContain(`- Exec approvals: ${sourcePath}${targetPath}`);
@@ -2458,6 +2459,7 @@ describe("doctor legacy state migrations", () => {
cfg: {},
env: { OPENCLAW_STATE_DIR: stateDir } as NodeJS.ProcessEnv,
homedir: () => root,
crossStateDirImports: true,
});
writeJson5(targetPath, {
version: 1,
@@ -2486,7 +2488,7 @@ describe("doctor legacy state migrations", () => {
});
});
it("auto-migrates exec approvals without a valid config snapshot", async () => {
it("auto-migrates exec approvals without a valid config snapshot when doctor opts in", async () => {
const root = await makeTempRoot();
const stateDir = path.join(root, "custom-state");
const sourcePath = path.join(root, ".openclaw", "exec-approvals.json");
@@ -2505,6 +2507,7 @@ describe("doctor legacy state migrations", () => {
const result = await autoMigrateLegacyTaskStateSidecars({
env: { OPENCLAW_STATE_DIR: stateDir } as NodeJS.ProcessEnv,
homedir: () => root,
crossStateDirImports: true,
});
expect(result.warnings).toStrictEqual([]);
@@ -2523,6 +2526,74 @@ describe("doctor legacy state migrations", () => {
});
});
it("keeps default exec approvals in place without the cross-state-dir opt-in", async () => {
// Regression: the implicit preflight (every CLI command, gateway startup)
// must never archive files that belong to the default state dir just
// because OPENCLAW_STATE_DIR points somewhere else.
const root = await makeTempRoot();
const stateDir = path.join(root, "custom-state");
const sourcePath = path.join(root, ".openclaw", "exec-approvals.json");
const targetPath = path.join(stateDir, "exec-approvals.json");
writeJson5(sourcePath, {
version: 1,
socket: {
token: "legacy-token",
},
defaults: {
security: "deny",
ask: "always",
},
});
const sourceRaw = fs.readFileSync(sourcePath, "utf8");
const result = await autoMigrateLegacyTaskStateSidecars({
env: { OPENCLAW_STATE_DIR: stateDir } as NodeJS.ProcessEnv,
homedir: () => root,
});
expect(result.warnings).toStrictEqual([]);
expect(result.changes).not.toContain(`Migrated exec approvals → ${targetPath}`);
expect(result.notices?.join("\n")).toContain(
"Exec approvals in the default state dir were not imported",
);
expect(fs.readFileSync(sourcePath, "utf8")).toBe(sourceRaw);
expect(fs.existsSync(`${sourcePath}.migrated`)).toBe(false);
expect(fs.existsSync(targetPath)).toBe(false);
});
it("keeps default exec approvals in place when autoMigrateLegacyState runs without the opt-in", async () => {
const root = await makeTempRoot();
const stateDir = path.join(root, "custom-state");
const sourcePath = path.join(root, ".openclaw", "exec-approvals.json");
const targetPath = path.join(stateDir, "exec-approvals.json");
writeJson5(sourcePath, {
version: 1,
socket: {
token: "legacy-token",
},
defaults: {
security: "deny",
},
});
const sourceRaw = fs.readFileSync(sourcePath, "utf8");
const result = await autoMigrateLegacyState({
cfg: {},
env: { OPENCLAW_STATE_DIR: stateDir } as NodeJS.ProcessEnv,
homedir: () => root,
log: { info: vi.fn(), warn: vi.fn() },
});
expect(result.warnings).toStrictEqual([]);
expect(result.changes).not.toContain(`Migrated exec approvals → ${targetPath}`);
expect(result.notices?.join("\n")).toContain(
"Exec approvals in the default state dir were not imported",
);
expect(fs.readFileSync(sourcePath, "utf8")).toBe(sourceRaw);
expect(fs.existsSync(`${sourcePath}.migrated`)).toBe(false);
expect(fs.existsSync(targetPath)).toBe(false);
});
it("keeps the plugin-state sidecar when shared state already has a conflicting row", async () => {
const root = await makeTempRoot();
const sourcePath = writeLegacyPluginStateSidecar(root);
+1
View File
@@ -286,6 +286,7 @@ function createLegacyStateMigrationDetectionResult(params?: {
plans: [],
},
warnings: [],
notices: [],
preview: params?.preview ?? [],
};
}
@@ -589,7 +589,7 @@ describe("doctor health contributions", () => {
mocks.noteChromeMcpBrowserReadiness.mockReset();
mocks.noteChromeMcpBrowserReadiness.mockResolvedValue(undefined);
mocks.detectLegacyStateMigrations.mockReset();
mocks.detectLegacyStateMigrations.mockResolvedValue({ preview: [], warnings: [] });
mocks.detectLegacyStateMigrations.mockResolvedValue({ preview: [], warnings: [], notices: [] });
mocks.runLegacyStateMigrations.mockReset();
mocks.runLegacyStateMigrations.mockResolvedValue({ changes: [], warnings: [] });
mocks.detectLegacyClawdBrowserProfileResidue.mockReset();
@@ -1434,7 +1434,7 @@ describe("doctor health contributions", () => {
expect(legacyStateCheck).toMatchObject({ defaultEnabled: false });
const cfg = { session: { store: "/tmp/shared-sessions.json" } };
const detected = { preview: ["legacy sessions"], warnings: [] };
const detected = { preview: ["legacy sessions"], warnings: [], notices: [] };
mocks.detectLegacyStateMigrations.mockResolvedValue(detected);
const ctx = {
cfg,
@@ -1455,7 +1455,7 @@ describe("doctor health contributions", () => {
it("prints legacy state migration notices during manual doctor runs", async () => {
const contribution = requireDoctorContribution("doctor:legacy-state");
const detected = { preview: ["legacy sessions"], warnings: [] };
const detected = { preview: ["legacy sessions"], warnings: [], notices: [] };
mocks.detectLegacyStateMigrations.mockResolvedValue(detected);
mocks.runLegacyStateMigrations.mockResolvedValue({
changes: [],
+14 -1
View File
@@ -518,10 +518,23 @@ async function runLegacyStateHealth(ctx: DoctorHealthFlowContext): Promise<void>
const { detectLegacyStateMigrations, runLegacyStateMigrations } =
await import("../commands/doctor-state-migrations.js");
const { note } = await loadNoteModule();
const legacyState = await detectLegacyStateMigrations({ cfg: ctx.cfg });
// 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.
const legacyState = await detectLegacyStateMigrations({
cfg: ctx.cfg,
crossStateDirImports:
ctx.options.nonInteractive !== true ||
ctx.options.repair === true ||
ctx.options.yes === true,
});
if (legacyState.warnings.length > 0) {
note(legacyState.warnings.join("\n"), "Doctor warnings");
}
if (legacyState.notices.length > 0) {
note(legacyState.notices.join("\n"), "Doctor notices");
}
if (legacyState.preview.length === 0) {
return;
}
+53 -2
View File
@@ -2313,7 +2313,7 @@ describe("state migrations", () => {
);
});
it("migrates legacy plugin binding approvals from the home state dir when using a custom state dir", async () => {
it("migrates home-state-dir plugin binding approvals only with the cross-state-dir opt-in", async () => {
const root = await createTempDir();
const stateDir = path.join(root, "custom-state");
const env = createEnv(stateDir);
@@ -2337,7 +2337,12 @@ describe("state migrations", () => {
"utf8",
);
const detected = await detectLegacyStateMigrations({ cfg, env, homedir: () => root });
const detected = await detectLegacyStateMigrations({
cfg,
env,
homedir: () => root,
crossStateDirImports: true,
});
expect(detected.pluginBindingApprovals).toMatchObject({
sourcePath,
hasLegacy: true,
@@ -2360,6 +2365,52 @@ describe("state migrations", () => {
await expectMissingPath(sourcePath);
});
it("leaves home-state-dir plugin binding approvals alone without the cross-state-dir opt-in", async () => {
// Regression: an isolated gateway pointed at a temp OPENCLAW_STATE_DIR must
// not import (and archive) the default state dir's approvals file.
const root = await createTempDir();
const stateDir = path.join(root, "custom-state");
const env = createEnv(stateDir);
const cfg = createConfig();
const sourcePath = path.join(root, ".openclaw", "plugin-binding-approvals.json");
const sourceRaw = JSON.stringify({
version: 1,
approvals: [
{
pluginRoot: "/plugins/codex-a",
pluginId: "codex",
channel: "telegram",
accountId: "default",
approvedAt: 2345,
},
],
});
await fs.mkdir(path.dirname(sourcePath), { recursive: true });
await fs.writeFile(sourcePath, sourceRaw, "utf8");
const detected = await detectLegacyStateMigrations({ cfg, env, homedir: () => root });
expect(detected.pluginBindingApprovals).toMatchObject({
sourcePath,
hasLegacy: false,
});
expect(detected.notices.join("\n")).toContain(
"Plugin binding approvals in the default state dir were not imported",
);
expect(detected.preview).not.toContain(
"- Plugin binding approvals: legacy JSON file → shared SQLite state",
);
const result = await runLegacyStateMigrations({ detected, config: cfg });
expect(result.warnings).toStrictEqual([]);
expect(result.changes).not.toContain(
"Migrated 1 plugin binding approval → shared SQLite state",
);
expect(readPluginBindingApprovalRows(env)).toEqual([]);
await expect(fs.readFile(sourcePath, "utf8")).resolves.toBe(sourceRaw);
await expectMissingPath(`${sourcePath}.migrated`);
});
it("imports non-conflicting legacy current-conversation bindings when SQLite has a conflict", async () => {
const root = await createTempDir();
const stateDir = path.join(root, ".openclaw");
+71 -11
View File
@@ -187,6 +187,7 @@ export type LegacyStateDetection = {
hasLegacy: boolean;
};
warnings: string[];
notices: string[];
preview: string[];
};
@@ -2404,7 +2405,9 @@ function migrateLegacyPluginBindingApprovals(params: {
}): { changes: string[]; warnings: string[] } {
const changes: string[] = [];
const warnings: string[] = [];
if (!fileExists(params.detected.sourcePath)) {
// hasLegacy is the detection verdict (it stays false for suppressed
// cross-state-dir sources); fileExists re-checks for races since detection.
if (!params.detected.hasLegacy || !fileExists(params.detected.sourcePath)) {
return { changes, warnings };
}
let approvals: LegacyPluginBindingApprovalEntry[];
@@ -4044,6 +4047,7 @@ export async function autoMigrateLegacyTaskStateSidecars(params: {
env?: NodeJS.ProcessEnv;
homedir?: () => string;
log?: MigrationLogger;
crossStateDirImports?: boolean;
}): Promise<{
migrated: boolean;
skipped: boolean;
@@ -4058,13 +4062,24 @@ export async function autoMigrateLegacyTaskStateSidecars(params: {
const stateDir = resolveStateDir(params.env ?? process.env, params.homedir);
const result = await migrateLegacyTaskStateSidecars({ stateDir });
const detectedExecApprovals = detectLegacyExecApprovalsMigration({
env: params.env ?? process.env,
homedir: params.homedir ?? os.homedir,
stateDir,
});
// Cross-state-dir sources need the explicit doctor opt-in (see
// detectLegacyStateMigrations); the implicit preflight must not archive
// files that belong to the default state dir.
const crossStateDirImports = params.crossStateDirImports === true;
const execApprovals = migrateLegacyExecApprovals(
detectLegacyExecApprovalsMigration({
env: params.env ?? process.env,
homedir: params.homedir ?? os.homedir,
stateDir,
}),
crossStateDirImports ? detectedExecApprovals : { ...detectedExecApprovals, hasLegacy: false },
);
const notices: string[] = [];
if (detectedExecApprovals.hasLegacy && !crossStateDirImports) {
notices.push(
`Exec approvals in the default state dir were not imported into OPENCLAW_STATE_DIR automatically (${detectedExecApprovals.sourcePath} -> ${detectedExecApprovals.targetPath}); run \`openclaw doctor --fix\` to import them.`,
);
}
const changes = [...result.changes, ...execApprovals.changes];
const warnings = [...result.warnings, ...execApprovals.warnings];
const logger = params.log ?? createSubsystemLogger("state-migrations");
@@ -4076,11 +4091,17 @@ export async function autoMigrateLegacyTaskStateSidecars(params: {
`Legacy state migration warnings:\n${warnings.map((entry) => `- ${entry}`).join("\n")}`,
);
}
if (notices.length > 0) {
logger.info(
`Legacy state migration notes:\n${notices.map((entry) => `- ${entry}`).join("\n")}`,
);
}
return {
migrated: changes.length > 0,
skipped: false,
changes,
warnings,
...(notices.length > 0 ? { notices } : {}),
};
}
@@ -4173,12 +4194,27 @@ export async function detectLegacyStateMigrations(params: {
homedir?: () => string;
pluginSessionStoreAgentIds?: readonly string[];
sessionStoreOwnership?: SessionStoreOwnership;
crossStateDirImports?: boolean;
}): Promise<LegacyStateDetection> {
const env = params.env ?? process.env;
const homedir = params.homedir ?? os.homedir;
const stateDir = resolveStateDir(env, homedir);
const oauthDir = resolveOAuthDir(env, stateDir);
const execApprovals = detectLegacyExecApprovalsMigration({ env, homedir, stateDir });
// Sources under the DEFAULT home state dir are foreign state when
// OPENCLAW_STATE_DIR points elsewhere: an isolated/test gateway must never
// import (and archive) another install's files. Only an explicit doctor run
// opts into the cross-directory import.
const crossStateDirImports = params.crossStateDirImports === true;
const notices: string[] = [];
const detectedExecApprovals = detectLegacyExecApprovalsMigration({ env, homedir, stateDir });
const execApprovals = crossStateDirImports
? detectedExecApprovals
: { ...detectedExecApprovals, hasLegacy: false };
if (detectedExecApprovals.hasLegacy && !crossStateDirImports) {
notices.push(
`Exec approvals in the default state dir were not imported into OPENCLAW_STATE_DIR automatically (${detectedExecApprovals.sourcePath} -> ${detectedExecApprovals.targetPath}); run \`openclaw doctor --fix\` to import them.`,
);
}
const targetAgentId = normalizeAgentId(resolveDefaultAgentId(params.cfg));
const rawMainKey = params.cfg.session?.mainKey;
@@ -4305,7 +4341,20 @@ export async function detectLegacyStateMigrations(params: {
const pluginBindingApprovals = {
sourcePath: resolveLegacyPluginBindingApprovalsPath(env, homedir),
};
const hasPluginBindingApprovals = fileExists(pluginBindingApprovals.sourcePath);
const pluginBindingApprovalsCrossDir =
path.resolve(path.dirname(pluginBindingApprovals.sourcePath)) !== path.resolve(stateDir);
const hasPluginBindingApprovals =
fileExists(pluginBindingApprovals.sourcePath) &&
(crossStateDirImports || !pluginBindingApprovalsCrossDir);
if (
fileExists(pluginBindingApprovals.sourcePath) &&
pluginBindingApprovalsCrossDir &&
!crossStateDirImports
) {
notices.push(
`Plugin binding approvals in the default state dir were not imported into OPENCLAW_STATE_DIR automatically (${pluginBindingApprovals.sourcePath}); run \`openclaw doctor --fix\` to import them.`,
);
}
const currentConversationBindings = {
sourcePath: resolveLegacyCurrentConversationBindingsPath(stateDir),
};
@@ -4473,6 +4522,7 @@ export async function detectLegacyStateMigrations(params: {
},
execApprovals,
warnings: pluginPlanWarnings,
notices,
preview,
};
}
@@ -5834,6 +5884,7 @@ export async function autoMigrateLegacyState(params: {
log?: MigrationLogger;
now?: () => number;
recoverCorruptTargetStore?: boolean;
crossStateDirImports?: boolean;
}): Promise<{
migrated: boolean;
skipped: boolean;
@@ -5912,6 +5963,7 @@ export async function autoMigrateLegacyState(params: {
sessionStoreOwnership,
env,
homedir: params.homedir,
crossStateDirImports: params.crossStateDirImports,
});
const hasCustomAgentDir = env.OPENCLAW_AGENT_DIR?.trim() || env.PI_CODING_AGENT_DIR?.trim();
if (hasCustomAgentDir) {
@@ -5999,7 +6051,11 @@ export async function autoMigrateLegacyState(params: {
...preSessionChannelPlans.warnings,
...pluginPlans.warnings,
];
const notices = [...(stateDirResult.notices ?? []), ...(pluginPlans.notices ?? [])];
const notices = [
...(stateDirResult.notices ?? []),
...detected.notices,
...(pluginPlans.notices ?? []),
];
logMigrationResults(changes, warnings, notices);
return {
migrated:
@@ -6057,7 +6113,7 @@ export async function autoMigrateLegacyState(params: {
...orphanKeys.warnings,
...acpSessionMetadata.warnings,
];
const notices = [...(stateDirResult.notices ?? [])];
const notices = [...(stateDirResult.notices ?? []), ...detected.notices];
logMigrationResults(changes, warnings, notices);
return {
migrated:
@@ -6178,7 +6234,11 @@ export async function autoMigrateLegacyState(params: {
...agentDir.warnings,
...channelPlans.warnings,
];
const notices = [...(stateDirResult.notices ?? []), ...(pluginPlans.notices ?? [])];
const notices = [
...(stateDirResult.notices ?? []),
...detected.notices,
...(pluginPlans.notices ?? []),
];
logMigrationResults(changes, warnings, notices);