proof: validate PR 133773 candidate

src/commands/doctor-config-preflight.process.test.ts

@@ -283,6 +283,120 @@ describe("doctor invalid config process exit", () => {
   }, 75_000);
 });
 
+describe("doctor legacy exec approvals process repair", () => {
+  it("imports legacy approvals before renewing obsolete generated grants", async () => {
+    const root = fs.realpathSync(tempDirs.make("openclaw-doctor-exec-approvals-order-"));
+    const stateDir = path.join(root, "state");
+    const configPath = path.join(stateDir, "openclaw.json");
+    const sourcePath = path.join(stateDir, "exec-approvals.json");
+    const databasePath = path.join(stateDir, "state", "openclaw.sqlite");
+    const env: NodeJS.ProcessEnv = {
+      ...process.env,
+      HOME: root,
+      USERPROFILE: root,
+      OPENCLAW_CONFIG_PATH: configPath,
+      OPENCLAW_DISABLE_BUNDLED_PLUGINS: "1",
+      OPENCLAW_NO_RESPAWN: "1",
+      OPENCLAW_SKIP_CHANNELS: "1",
+      OPENCLAW_STATE_DIR: stateDir,
+      OPENCLAW_TEST_FAST: "1",
+      NO_COLOR: "1",
+    };
+    delete env.NODE_ENV;
+    delete env.NODE_OPTIONS;
+    delete env.OPENCLAW_GATEWAY_PASSWORD;
+    delete env.OPENCLAW_GATEWAY_TOKEN;
+    delete env.OPENCLAW_GATEWAY_URL;
+    delete env.OPENCLAW_HOME;
+    delete env.VITEST;
+    delete env.VITEST_POOL_ID;
+    delete env.VITEST_WORKER_ID;
+
+    fs.mkdirSync(stateDir, { recursive: true });
+    fs.writeFileSync(
+      configPath,
+      JSON.stringify({ gateway: { mode: "local", auth: { mode: "none" } } }),
+    );
+    fs.writeFileSync(
+      sourcePath,
+      JSON.stringify({
+        version: 1,
+        agents: {
+          main: {
+            allowlist: [
+              { pattern: "/usr/bin/rg", source: "allow-always" },
+              { pattern: "=command:durable", source: "allow-always" },
+            ],
+          },
+        },
+      }),
+    );
+    const preflightUrl = new URL("./doctor-config-preflight.ts", import.meta.url).href;
+    await runIsolatedModuleScript(
+      env,
+      `
+        const { runDoctorConfigPreflight } = await import(${JSON.stringify(preflightUrl)});
+        await runDoctorConfigPreflight({
+          migrateLegacyConfig: false,
+          invalidConfigNote: false,
+          observe: false,
+          requireStartupMigrationCheckpoint: true,
+          beforeStateMigrations: async () => true,
+        });
+      `,
+    );
+    expect(fs.existsSync(sourcePath)).toBe(true);
+
+    const result = spawnSync(
+      process.execPath,
+      [
+        "--import",
+        "tsx",
+        path.resolve("src/entry.ts"),
+        "doctor",
+        "--fix",
+        "--non-interactive",
+        "--no-workspace-suggestions",
+      ],
+      {
+        cwd: path.resolve("."),
+        encoding: "utf8",
+        env,
+        timeout: 60_000,
+      },
+    );
+    const output = `${result.stderr}\n${result.stdout}`;
+
+    expect(result.error, output).toBeUndefined();
+    expect(result.status, output).toBe(0);
+    expect(result.signal, output).toBeNull();
+    expect(output).toContain("Doctor complete.");
+    expect(fs.existsSync(sourcePath)).toBe(false);
+
+    const database = new DatabaseSync(databasePath, { readOnly: true });
+    try {
+      const row = database
+        .prepare("SELECT raw_json FROM exec_approvals_config WHERE config_key = 'current'")
+        .get() as { raw_json: string } | undefined;
+      const approvals = JSON.parse(row?.raw_json ?? "null") as {
+        agents?: { main?: { allowlist?: Array<{ pattern?: string }> } };
+      } | null;
+      expect(approvals?.agents?.main?.allowlist).toEqual([
+        expect.objectContaining({ pattern: "=command:durable" }),
+      ]);
+      expect(
+        database
+          .prepare(
+            "SELECT status, removed_source FROM migration_sources WHERE migration_kind = 'legacy-exec-approvals-json'",
+          )
+          .get(),
+      ).toMatchObject({ status: "completed", removed_source: 1 });
+    } finally {
+      database.close();
+    }
+  }, 75_000);
+});
+
 // Synchronous CLI probes must not consume neighboring cases' timeout budgets.
 describe("gateway startup-migration refusal", () => {
   it("repairs the stable upgrade config and additive state schema before readiness", async () => {

src/commands/doctor-security.test.ts

@@ -1,4 +1,5 @@
 // Doctor security tests cover security audit checks, config findings, and repair output.
+import fs from "node:fs/promises";
 import path from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import type { OpenClawConfig } from "../config/config.js";
@@ -88,6 +89,30 @@ describe("noteSecurityWarnings gateway exposure", () => {
 
   const lastMessage = () => String(note.mock.calls[note.mock.calls.length - 1]?.[0] ?? "");
 
+  it("does not let pending legacy exec approvals abort Doctor security checks", async () => {
+    await withTestDir({ prefix: "openclaw-doctor-security-legacy-" }, async (home) => {
+      const stateDir = path.join(home, ".openclaw");
+      process.env.HOME = home;
+      process.env.OPENCLAW_STATE_DIR = stateDir;
+      await fs.mkdir(stateDir, { recursive: true });
+      await fs.writeFile(
+        path.join(stateDir, "exec-approvals.json"),
+        `${JSON.stringify({ version: 1 })}\n`,
+        "utf8",
+      );
+      closeOpenClawStateDatabaseForTest();
+      execApprovalsStoreTesting.reset();
+
+      const findings = await collectSecurityWarnings({});
+
+      expect(findings).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({ checkId: "doctor.exec_approvals_migration_pending" }),
+        ]),
+      );
+    });
+  });
+
   async function withExecApprovalsFile(
     file: Record<string, unknown>,
     run: () => Promise<void>,

src/commands/doctor-security.ts

@@ -11,9 +11,11 @@ import { resolveGatewayAuth } from "../gateway/auth.js";
 import { isLoopbackHost, resolveGatewayBindHost } from "../gateway/net.js";
 import { resolveExecPolicyScopeSnapshot } from "../infra/exec-approvals-effective.js";
 import { countObsoleteGeneratedExecApprovals } from "../infra/exec-approvals-generated-migration.js";
+import { ExecApprovalsMigrationRequiredError } from "../infra/exec-approvals-migration-gate.js";
 import {
   loadExecApprovalsReadOnly,
   resolveExecApprovalsDisplayPath,
+  type ExecApprovalsFile,
   type ExecAsk,
   type ExecMode,
   type ExecSecurity,
@@ -92,9 +94,11 @@ function execAskRank(value: ExecAsk): number {
   throw new Error("Unsupported exec ask value");
 }
 
-function collectExecPolicyConflictWarnings(cfg: OpenClawConfig): SecurityAuditFinding[] {
+function collectExecPolicyConflictWarnings(
+  cfg: OpenClawConfig,
+  approvals: ExecApprovalsFile,
+): SecurityAuditFinding[] {
   const findings: SecurityAuditFinding[] = [];
-  const approvals = loadExecApprovalsReadOnly();
   const defaultRequestedSecuritySource = "OpenClaw default (full)";
   const defaultRequestedAskSource = "OpenClaw default (off)";
 
@@ -194,9 +198,12 @@ function collectExecPolicyConflictWarnings(cfg: OpenClawConfig): SecurityAuditFi
   return findings;
 }
 
-function collectDurableExecApprovalWarnings(cfg: OpenClawConfig): SecurityAuditFinding[] {
+function collectDurableExecApprovalWarnings(
+  cfg: OpenClawConfig,
+  approvals: ExecApprovalsFile,
+): SecurityAuditFinding[] {
   void cfg;
-  const count = countObsoleteGeneratedExecApprovals(loadExecApprovalsReadOnly());
+  const count = countObsoleteGeneratedExecApprovals(approvals);
   if (count === 0) {
     return [];
   }
@@ -303,10 +310,32 @@ export async function collectSecurityWarnings(
   }
 
   findings.push(...collectImplicitHeartbeatDirectPolicyWarnings(cfg));
-  findings.push(...collectExecPolicyConflictWarnings(cfg));
+  let approvals: ExecApprovalsFile | undefined;
+  try {
+    approvals = loadExecApprovalsReadOnly();
+  } catch (error) {
+    if (!(error instanceof ExecApprovalsMigrationRequiredError)) {
+      throw error;
+    }
+    // Doctor owns this migration later in the same repair flow. Security
+    // diagnostics must not invoke the runtime gate first and make
+    // `doctor --fix` abort with an instruction to run itself.
+    findings.push({
+      checkId: "doctor.exec_approvals_migration_pending",
+      severity: "warn",
+      title: "Exec approvals migration pending",
+      detail: error.message,
+      remediation: "Continue this Doctor repair to migrate the retired approvals store.",
+    });
+  }
+  if (approvals) {
+    findings.push(...collectExecPolicyConflictWarnings(cfg, approvals));
+  }
   findings.push(...collectExecFilesystemPolicyWarnings(cfg));
   findings.push(...collectPlaintextConfigSecretWarnings(cfg));
-  findings.push(...collectDurableExecApprovalWarnings(cfg));
+  if (approvals) {
+    findings.push(...collectDurableExecApprovalWarnings(cfg, approvals));
+  }
 
   // Network exposure needs auth proof before doctor can treat non-loopback bind as intentional.
   const tailscaleMode = cfg.gateway?.tailscale?.mode ?? "off";

src/commands/doctor/repair-sequencing.ts

@@ -5,7 +5,6 @@ import {
   applyPluginAutoEnable,
   materializePluginAutoEnableCandidates,
 } from "../../config/plugin-auto-enable.js";
-import { repairObsoleteGeneratedExecApprovals } from "../../infra/exec-approvals-generated-migration.js";
 import type { PluginCapabilityConsentHandler } from "../../plugins/capability-consent.js";
 import type { PluginMetadataSnapshotScopeRunner } from "../../plugins/current-plugin-metadata-snapshot.js";
 import {
@@ -112,13 +111,6 @@ export async function runDoctorRepairSequence(params: {
     return params.runWithPluginMetadataSnapshot(resolveCurrentPluginMetadataScope(), run);
   };
 
-  const removedExecApprovals = repairObsoleteGeneratedExecApprovals();
-  if (removedExecApprovals > 0) {
-    changeNotes.push(
-      `Exec approvals updated: removed ${removedExecApprovals} older generated ${removedExecApprovals === 1 ? "approval" : "approvals"} that were not tied to a working directory. Manual allowlist rules were not changed. Rerun affected workflows and choose "Always allow here" when prompted.`,
-    );
-  }
-
   const applyMutation = (mutation: {
     config: DoctorConfigMutationState["candidate"];
     changes: string[];

src/flows/doctor-health-contributions.test.ts

@@ -107,6 +107,7 @@ const mocks = vi.hoisted(() => ({
   noteChromeMcpBrowserReadiness: vi.fn(),
   detectLegacyStateMigrations: vi.fn(),
   runLegacyStateMigrations: vi.fn(),
+  repairObsoleteGeneratedExecApprovals: vi.fn(() => 0),
   collectLegacyPluginManifestContractMigrations: vi.fn(() => [] as unknown[]),
   legacyPluginManifestContractMigrationToHealthFinding: vi.fn(
     (migration: { pluginId: string }) => ({
@@ -339,6 +340,10 @@ vi.mock("../infra/state-migrations.doctor.js", () => ({
   runLegacyStateMigrations: mocks.runLegacyStateMigrations,
 }));
 
+vi.mock("../infra/exec-approvals-generated-migration.js", () => ({
+  repairObsoleteGeneratedExecApprovals: mocks.repairObsoleteGeneratedExecApprovals,
+}));
+
 vi.mock("../commands/doctor-plugin-manifests.js", () => ({
   collectLegacyPluginManifestContractMigrations:
     mocks.collectLegacyPluginManifestContractMigrations,
@@ -757,6 +762,7 @@ describe("doctor health contributions", () => {
       .mockReset()
       .mockResolvedValue({ preview: [], warnings: [], notices: [] });
     mocks.runLegacyStateMigrations.mockReset().mockResolvedValue({ changes: [], warnings: [] });
+    mocks.repairObsoleteGeneratedExecApprovals.mockReset().mockReturnValue(0);
     mocks.detectLegacyClawdBrowserProfileResidue.mockReset().mockReturnValue(null);
     mocks.maybeArchiveLegacyClawdBrowserProfileResidue.mockReset().mockResolvedValue({
       changes: [],
@@ -2197,6 +2203,26 @@ describe("doctor health contributions", () => {
     });
   });
 
+  it("still renews generated exec approvals after declining an unrelated migration", async () => {
+    const contribution = requireDoctorContribution("doctor:legacy-state");
+    mocks.detectLegacyStateMigrations.mockResolvedValue({
+      preview: ["legacy sessions"],
+      warnings: [],
+      notices: [],
+    });
+    const ctx = createDoctorContext({
+      cfg: {},
+      configResult: {},
+      shouldRepair: false,
+      options: { repair: true },
+    });
+
+    await contribution.run(ctx);
+
+    expect(mocks.runLegacyStateMigrations).not.toHaveBeenCalled();
+    expect(mocks.repairObsoleteGeneratedExecApprovals).toHaveBeenCalledOnce();
+  });
+
   it("prints legacy state migration notices during manual doctor runs", async () => {
     const contribution = requireDoctorContribution("doctor:legacy-state");
     const detected = { preview: ["legacy sessions"], warnings: [], notices: [] };

src/flows/doctor-health-contributions.ts

@@ -284,36 +284,57 @@ async function runLegacyStateHealth(ctx: DoctorHealthFlowContext): Promise<void>
   if (legacyState.notices.length > 0) {
     note(legacyState.notices.join("\n"), "Doctor notices");
   }
-  if (legacyState.preview.length === 0) {
-    return;
+  if (legacyState.preview.length > 0) {
+    note(legacyState.preview.join("\n"), "Legacy state detected");
+    const migrate =
+      ctx.options.nonInteractive === true
+        ? true
+        : await ctx.prompter.confirm({
+            message: "Migrate detected legacy state now?",
+            initialValue: true,
+          });
+    if (migrate) {
+      const migrated = await runLegacyStateMigrations({
+        detected: legacyState,
+        config: ctx.cfg,
+        ...(doctorOnlyStateMigrations ? { doctorOnlyStateMigrations: true } : {}),
+        recoverCorruptTargetStore: ctx.options.repair === true || ctx.options.yes === true,
+        legacySessionSurfaces,
+      });
+      if (migrated.changes.length > 0) {
+        note(migrated.changes.join("\n"), "Doctor changes");
+      }
+      const notices = migrated.notices ?? [];
+      if (notices.length > 0) {
+        note(notices.join("\n"), "Doctor notices");
+      }
+      if (migrated.warnings.length > 0) {
+        note(migrated.warnings.join("\n"), "Doctor warnings");
+      }
+    }
   }
-  note(legacyState.preview.join("\n"), "Legacy state detected");
-  const migrate =
-    ctx.options.nonInteractive === true
-      ? true
-      : await ctx.prompter.confirm({
-          message: "Migrate detected legacy state now?",
-          initialValue: true,
-        });
-  if (!migrate) {
+  if (!doctorOnlyStateMigrations) {
     return;
   }
-  const migrated = await runLegacyStateMigrations({
-    detected: legacyState,
-    config: ctx.cfg,
-    ...(doctorOnlyStateMigrations ? { doctorOnlyStateMigrations: true } : {}),
-    recoverCorruptTargetStore: ctx.options.repair === true || ctx.options.yes === true,
-    legacySessionSurfaces,
-  });
-  if (migrated.changes.length > 0) {
-    note(migrated.changes.join("\n"), "Doctor changes");
-  }
-  const notices = migrated.notices ?? [];
-  if (notices.length > 0) {
-    note(notices.join("\n"), "Doctor notices");
+  const { repairObsoleteGeneratedExecApprovals } =
+    await import("../infra/exec-approvals-generated-migration.js");
+  const { ExecApprovalsMigrationRequiredError } =
+    await import("../infra/exec-approvals-migration-gate.js");
+  let removedExecApprovals: number;
+  try {
+    // The legacy-state owner must import retired JSON before this gated SQLite update.
+    removedExecApprovals = repairObsoleteGeneratedExecApprovals();
+  } catch (error) {
+    if (error instanceof ExecApprovalsMigrationRequiredError) {
+      return;
+    }
+    throw error;
   }
-  if (migrated.warnings.length > 0) {
-    note(migrated.warnings.join("\n"), "Doctor warnings");
+  if (removedExecApprovals > 0) {
+    note(
+      `Exec approvals updated: removed ${removedExecApprovals} older generated ${removedExecApprovals === 1 ? "approval" : "approvals"} that were not tied to a working directory. Manual allowlist rules were not changed. Rerun affected workflows and choose "Always allow here" when prompted.`,
+      "Doctor changes",
+    );
   }
 }