diff --git a/src/lib/actions/sandbox/messaging-host-forward-lifecycle.ts b/src/lib/actions/sandbox/messaging-host-forward-lifecycle.ts index 07b5b52065a..bf945049a82 100644 --- a/src/lib/actions/sandbox/messaging-host-forward-lifecycle.ts +++ b/src/lib/actions/sandbox/messaging-host-forward-lifecycle.ts @@ -45,6 +45,7 @@ export function ensureMessagingHostForwardAfterRebuild( sandboxName: string, plan: SandboxMessagingPlan | null | undefined, note: (message: string) => void = console.log, + revalidatePolicyAuthority?: (operation: string) => void, ): boolean { const forward = resolveMessagingHostForward(plan); if (!forward) return true; @@ -67,6 +68,7 @@ export function ensureMessagingHostForwardAfterRebuild( name, port, label, + revalidatePolicyAuthority, ), note, }); diff --git a/src/lib/actions/sandbox/policy-channel-agent-gate.test.ts b/src/lib/actions/sandbox/policy-channel-agent-gate.test.ts index 984e78e45f1..95ba2b16dc7 100644 --- a/src/lib/actions/sandbox/policy-channel-agent-gate.test.ts +++ b/src/lib/actions/sandbox/policy-channel-agent-gate.test.ts @@ -16,6 +16,7 @@ import * as policy from "../../policy"; import * as registry from "../../state/registry"; import { addSandboxChannel, startSandboxChannel, stopSandboxChannel } from "./policy-channel"; import { policyChannelDependencies } from "./policy-channel-dependencies"; +import * as policyAuthorityPreflight from "./policy-authority/preflight"; function agentFixture(name: string): defs.AgentDefinition { return { name } as defs.AgentDefinition; @@ -76,6 +77,9 @@ beforeEach(() => { saveCredentialMock = vi.spyOn(store, "saveCredential").mockImplementation(() => undefined); promptMock = vi.spyOn(store, "prompt").mockResolvedValue(""); rebuildMock = vi.spyOn(policyChannelDependencies, "rebuildSandbox").mockResolvedValue(undefined); + vi.spyOn(policyAuthorityPreflight, "preflightSandboxPolicyAuthority").mockReturnValue( + "nemoclaw-managed", + ); }); afterEach(() => { @@ -155,39 +159,42 @@ describe("channel lifecycle agent gate", () => { it.each([ ["start", ["googlechat"], () => startSandboxChannel("da-test", { channel: "googlechat" })], ["stop", [], () => stopSandboxChannel("da-test", { channel: "googlechat" })], - ])("rejects a stale channel during %s before reading channel state or mutating the sandbox", async (_verb, disabledChannels, run) => { - // googlechat now supports openclaw + hermes, so exercise the unsupported-pair - // lifecycle gate with a non-messaging custom agent (supported by no channel). - getSandboxMock.mockReturnValue({ name: "da-test", agent: "custom-agent" }); - vi.spyOn(defs, "loadAgent").mockReturnValue(agentFixture("custom-agent")); - const configuredChannelsMock = vi - .spyOn(registry, "getConfiguredMessagingChannelsFromEntry") - .mockReturnValue(["googlechat"]); - const disabledChannelsMock = vi - .spyOn(registry, "getDisabledChannels") - .mockReturnValue(disabledChannels); - - let caught: unknown; - try { - await run(); - } catch (err) { - caught = err; - } - - expect(exitCodeFromError(caught)).toBe(1); - const errorText = (errSpy.mock.calls as unknown[][]) - .map((call) => call.map(String).join(" ")) - .join("\n"); - expect(errorText).toMatch(/Channel 'googlechat' does not support agent 'custom-agent'/); - expect(errorText).toMatch(/Channel-supported agents: openclaw, hermes/); - expect(errorText).toMatch(/Channels supported by agent 'custom-agent': \(none\)/); - - expect(configuredChannelsMock).not.toHaveBeenCalled(); - expect(disabledChannelsMock).not.toHaveBeenCalled(); - expect(loadPresetForSandboxMock).not.toHaveBeenCalled(); - expect(applyPresetMock).not.toHaveBeenCalled(); - expect(updateSandboxMock).not.toHaveBeenCalled(); - expect(rebuildMock).not.toHaveBeenCalled(); - expect(runOpenshellMock).not.toHaveBeenCalled(); - }); + ])( + "rejects a stale channel during %s before reading channel state or mutating the sandbox", + async (_verb, disabledChannels, run) => { + // googlechat now supports openclaw + hermes, so exercise the unsupported-pair + // lifecycle gate with a non-messaging custom agent (supported by no channel). + getSandboxMock.mockReturnValue({ name: "da-test", agent: "custom-agent" }); + vi.spyOn(defs, "loadAgent").mockReturnValue(agentFixture("custom-agent")); + const configuredChannelsMock = vi + .spyOn(registry, "getConfiguredMessagingChannelsFromEntry") + .mockReturnValue(["googlechat"]); + const disabledChannelsMock = vi + .spyOn(registry, "getDisabledChannels") + .mockReturnValue(disabledChannels); + + let caught: unknown; + try { + await run(); + } catch (err) { + caught = err; + } + + expect(exitCodeFromError(caught)).toBe(1); + const errorText = (errSpy.mock.calls as unknown[][]) + .map((call) => call.map(String).join(" ")) + .join("\n"); + expect(errorText).toMatch(/Channel 'googlechat' does not support agent 'custom-agent'/); + expect(errorText).toMatch(/Channel-supported agents: openclaw, hermes/); + expect(errorText).toMatch(/Channels supported by agent 'custom-agent': \(none\)/); + + expect(configuredChannelsMock).not.toHaveBeenCalled(); + expect(disabledChannelsMock).not.toHaveBeenCalled(); + expect(loadPresetForSandboxMock).not.toHaveBeenCalled(); + expect(applyPresetMock).not.toHaveBeenCalled(); + expect(updateSandboxMock).not.toHaveBeenCalled(); + expect(rebuildMock).not.toHaveBeenCalled(); + expect(runOpenshellMock).not.toHaveBeenCalled(); + }, + ); }); diff --git a/src/lib/actions/sandbox/policy-channel-cleanup.test.ts b/src/lib/actions/sandbox/policy-channel-cleanup.test.ts index 99330046557..ceae9dd3ea1 100644 --- a/src/lib/actions/sandbox/policy-channel-cleanup.test.ts +++ b/src/lib/actions/sandbox/policy-channel-cleanup.test.ts @@ -4,7 +4,7 @@ // Cleanup-path regression: with the new strict `toMessagingAgentId` semantics, // stale messaging state on a non-messaging agent must still be cleanable // without raising MessagingAgentNotSupportedError. `channels remove` should -// strip the stored messaging plan from the registry, `channels pause/resume` +// strip the stored messaging plan from the registry, `channels stop/start` // should fail closed (no throw, no plan mutation). import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from "vitest"; diff --git a/src/lib/actions/sandbox/policy-channel-conflict.test.ts b/src/lib/actions/sandbox/policy-channel-conflict.test.ts index d3c871b4505..04379c6058f 100644 --- a/src/lib/actions/sandbox/policy-channel-conflict.test.ts +++ b/src/lib/actions/sandbox/policy-channel-conflict.test.ts @@ -8,10 +8,12 @@ // import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from "vitest"; +import { PolicyAuthorityRefusalError } from "../../adapters/openshell/policy-authority"; import * as runtime from "../../adapters/openshell/runtime"; import * as defs from "../../agent/defs"; import * as store from "../../credentials/store"; import * as gatewayRuntime from "../../gateway-runtime-action"; +import * as wechatLogin from "../../messaging/channels/wechat/login"; import * as policy from "../../policy"; import { hashCredential } from "../../security/credential-hash"; import * as onboardSession from "../../state/onboard-session"; @@ -39,7 +41,6 @@ function successfulOpenshellResult(): ReturnType { const TELEGRAM_TOKEN = "123456:AAH-secret-bot-token-value"; const TELEGRAM_HASH = hashCredential(TELEGRAM_TOKEN) as string; -const DISCORD_TOKEN = "discord-test-token"; // Build a minimal plan-backed SandboxEntry for conflict-detection fixtures. // Callers supply credential bindings as { providerEnvKey, credentialHash? }. @@ -282,6 +283,7 @@ let getSandboxMock: MockInstance; let getDisabledChannelsMock: MockInstance; let listSandboxesMock: MockInstance; let rebuildSandboxMock: MockInstance; +let stopSandboxMock: MockInstance; let ensureMessagingHostForwardAfterRebuildMock: MockInstance; let scopeDisclosureMock: MockInstance; @@ -323,11 +325,13 @@ beforeEach(() => { delete process.env.SLACK_APP_TOKEN; delete process.env.SLACK_ALLOWED_USERS; delete process.env.SLACK_ALLOWED_CHANNELS; - delete process.env.DISCORD_BOT_TOKEN; delete process.env.NEMOCLAW_SKIP_TELEGRAM_REACHABILITY; delete process.env.NEMOCLAW_SKIP_SLACK_AUTH_VALIDATION; delete process.env.WECHAT_BOT_TOKEN; delete process.env.WECHAT_ACCOUNT_ID; + vi.spyOn(policyChannelDependencies, "preflightSandboxPolicyAuthority").mockReturnValue( + "nemoclaw-managed", + ); delete process.env.WECHAT_BASE_URL; delete process.env.WECHAT_USER_ID; delete process.env.MSTEAMS_APP_ID; @@ -353,9 +357,6 @@ beforeEach(() => { // Lazy legacy-provider seam: no onboarding graph is loaded for this suite. upsertMock = vi.spyOn(policyChannelDependencies, "upsertMessagingProviders").mockReturnValue([]); - vi.spyOn(policyChannelDependencies, "revalidateChannelProviderPolicyAuthority").mockImplementation( - () => undefined, - ); // openshell runtime + gateway recovery. runOpenshellMock = vi.spyOn(runtime, "runOpenshell").mockReturnValue(successfulOpenshellResult()); @@ -393,12 +394,16 @@ beforeEach(() => { .spyOn(policy, "logPresetScopeForState") .mockImplementation(() => undefined); applyPresetMock = vi.spyOn(policy, "applyPreset").mockReturnValue(true); + vi.spyOn(policy, "removePreset").mockReturnValue(true); vi.spyOn(policy, "getAppliedPresets").mockReturnValue([]); // Downstream rebuild is not under test. rebuildSandboxMock = vi .spyOn(policyChannelDependencies, "rebuildSandbox") .mockResolvedValue(undefined); + stopSandboxMock = vi + .spyOn(policyChannelDependencies, "stopSandbox") + .mockReturnValue({ exitCode: 0 }); ensureMessagingHostForwardAfterRebuildMock = vi .spyOn(messagingHostForwardLifecycle, "ensureMessagingHostForwardAfterRebuild") .mockReturnValue(true); @@ -576,93 +581,6 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { expect(updateSandboxMock).toHaveBeenCalledWith("alpha", expect.any(Object)); }); - it("registers Hermes Discord with the exact static provider binding", async () => { - arrangeRegistry({ - current: { ...makeEmptyEntry("alpha"), agent: "hermes" } as SandboxEntry, - }); - vi.mocked(defs.loadAgent).mockReturnValue(agentFixture("hermes")); - getCredentialMock.mockImplementation((key: string) => - key === "DISCORD_BOT_TOKEN" ? DISCORD_TOKEN : null, - ); - - await addSandboxChannel("alpha", { channel: "discord" }); - - expect(upsertMock).toHaveBeenCalledWith( - [ - { - name: "alpha-discord-bridge", - envKey: "DISCORD_BOT_TOKEN", - token: DISCORD_TOKEN, - providerType: "discord-hermes-static-v1", - }, - ], - "nemoclaw", - { bestEffort: true, requireExactBindings: true }, - ); - }); - - it("does not remove a pre-existing provider after a Hermes Discord identity conflict", async () => { - const originalEntry = { ...makeEmptyEntry("alpha"), agent: "hermes" } as SandboxEntry; - arrangeRegistry({ current: originalEntry }); - vi.mocked(defs.loadAgent).mockReturnValue(agentFixture("hermes")); - getCredentialMock.mockImplementation((key: string) => - key === "DISCORD_BOT_TOKEN" ? DISCORD_TOKEN : null, - ); - upsertMock.mockImplementationOnce(() => { - throw Object.assign( - new Error("alpha-discord-bridge does not match the required binding"), - { - code: "NEMOCLAW_MESSAGING_PROVIDER_BINDING_CONFLICT", - mutatedProviderNames: [], - }, - ); - }); - - await expect(addSandboxChannel("alpha", { channel: "discord" })).rejects.toThrow( - "process.exit(1)", - ); - - expect(updateSandboxMock).not.toHaveBeenCalled(); - expect(registry.getSandbox("alpha")).toBe(originalEntry); - expect( - runOpenshellMock.mock.calls - .map(([args]) => (args as string[]).join(" ")) - .filter((command) => command.includes("provider detach") || command.includes("delete")), - ).toEqual([]); - }); - - it("does not persist a multi-provider add when identity preflight fails", async () => { - const originalEntry = makeEmptyEntry("alpha"); - arrangeRegistry({ current: originalEntry }); - const slackBot = "xoxb-alpha-slack-bot-token"; - const slackApp = "xapp-alpha-slack-app-token"; - getCredentialMock.mockImplementation((key: string) => - key === "SLACK_BOT_TOKEN" ? slackBot : key === "SLACK_APP_TOKEN" ? slackApp : null, - ); - upsertMock.mockImplementationOnce(() => { - throw Object.assign(new Error("alpha-slack-app does not match the required binding"), { - code: "NEMOCLAW_MESSAGING_PROVIDER_BINDING_CONFLICT", - mutatedProviderNames: [], - }); - }); - - await expect(addSandboxChannel("alpha", { channel: "slack" })).rejects.toThrow( - "process.exit(1)", - ); - - expect(upsertMock.mock.calls[0]?.[0]).toHaveLength(2); - expect(saveCredentialMock).not.toHaveBeenCalled(); - expect(applyPresetMock).not.toHaveBeenCalled(); - expect(updateSandboxMock).not.toHaveBeenCalled(); - expect(rebuildSandboxMock).not.toHaveBeenCalled(); - expect(registry.getSandbox("alpha")).toBe(originalEntry); - expect( - runOpenshellMock.mock.calls - .map(([args]) => (args as string[]).join(" ")) - .filter((command) => command.includes("provider detach") || command.includes("delete")), - ).toEqual([]); - }); - // Scenario 6 it("idempotent same-sandbox re-add does not self-conflict", async () => { arrangeRegistry({ @@ -708,6 +626,35 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { }); // Scenario 8: WeChat is host-qr (token-bearing) -> non-empty acquired -> IS conflict-checked. + it("stops host-QR credential persistence when policy authority changes during login (#9833)", async () => { + arrangeRegistry({ current: makeEmptyEntry("alpha") }); + const runLogin = vi.spyOn(wechatLogin, "runWechatHostQrLogin").mockImplementation(async () => { + vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority).mockImplementation( + () => { + throw new Error("OpenShell policy authority changed during QR login"); + }, + ); + return { + kind: "ok", + credentials: { + token: "wechat-token", + accountId: "wechat-account", + baseUrl: "https://ilinkai.wechat.com", + userId: "wechat-user", + }, + }; + }); + + await expect(addSandboxChannel("alpha", { channel: "wechat" })).rejects.toThrow( + "process.exit(1)", + ); + + expect(runLogin).toHaveBeenCalledOnce(); + expect(saveCredentialMock).not.toHaveBeenCalled(); + expect(process.env.WECHAT_BOT_TOKEN).toBeUndefined(); + expect(upsertMock).not.toHaveBeenCalled(); + }); + it("host-qr wechat (token-bearing) IS conflict-checked", async () => { const wechatToken = "wx-secret-token-abc"; const wechatHash = hashCredential(wechatToken) as string; @@ -767,6 +714,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { const text = loggedText(); expect(text).toContain("Could not persist messaging plan for 'alpha'"); expect(text).not.toContain("Enabled whatsapp channel"); + expect(updateSandboxMock).toHaveBeenLastCalledWith("alpha", { messaging: undefined }); expect(exitMock).toHaveBeenCalledWith(1); expect(promptMock).not.toHaveBeenCalled(); }); @@ -1179,6 +1127,37 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { ).toBe(true); }); + it("withholds bridge-health success when policy authority changes during the probe (#9833)", async () => { + arrangeRegistry({ current: { name: "alpha" } as SandboxEntry }); + getCredentialMock.mockImplementation((key: string) => + key === "TELEGRAM_BOT_TOKEN" ? TELEGRAM_TOKEN : null, + ); + const refusal = new PolicyAuthorityRefusalError( + "OpenShell policy authority changed during bridge verification", + ); + vi.mocked(processRecovery.executeSandboxExecCommand) + .mockReturnValueOnce({ + status: 0, + stdout: JSON.stringify({ channels: { telegram: { enabled: true } } }), + stderr: "", + }) + .mockImplementation(() => { + vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority).mockImplementation( + () => { + throw refusal; + }, + ); + return { status: 0, stdout: "[telegram] [default] starting provider\n", stderr: "" }; + }); + + await expect(addSandboxChannel("alpha", { channel: "telegram" })).rejects.toBe(refusal); + + expect(loggedText()).not.toContain("'telegram' bridge startup detected"); + expect(policy.removePreset).not.toHaveBeenCalled(); + expect(stopSandboxMock).toHaveBeenCalledOnce(); + expect(stopSandboxMock).toHaveBeenCalledWith("alpha"); + }); + it("runs Slack post-rebuild warning detection through the channel hook", async () => { arrangeRegistry({ current: { name: "alpha" } as SandboxEntry }); getCredentialMock.mockImplementation((key: string) => @@ -1234,6 +1213,8 @@ describe("Teams host-forward lifecycle (PRA-2)", () => { expect(ensureMessagingHostForwardAfterRebuildMock).toHaveBeenCalledWith( "alpha", expect.any(Object), + undefined, + expect.any(Function), ); expect(ensureMessagingHostForwardAfterRebuildMock.mock.invocationCallOrder[0]).toBeGreaterThan( rebuildSandboxMock.mock.invocationCallOrder[0], @@ -1245,6 +1226,36 @@ describe("Teams host-forward lifecycle (PRA-2)", () => { }); }); + it("channels add stops the rebuilt sandbox when host-forward authority validation fails (#9833)", async () => { + setTeamsEnv(); + arrangeRegistry({ current: makeEmptyEntry("alpha") }); + const refusal = new PolicyAuthorityRefusalError( + "OpenShell policy authority changed before the host forward started", + ); + let validatingHostForward = false; + vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority).mockImplementation(() => { + return validatingHostForward + ? (() => { + throw refusal; + })() + : "nemoclaw-managed"; + }); + ensureMessagingHostForwardAfterRebuildMock.mockImplementation( + (_sandboxName, _plan, _note, revalidatePolicyAuthority) => { + validatingHostForward = true; + revalidatePolicyAuthority?.("start the Microsoft Teams webhook forward"); + return true; + }, + ); + + await expect(addSandboxChannel("alpha", { channel: "teams" })).rejects.toBe(refusal); + + expect(rebuildSandboxMock).toHaveBeenCalledOnce(); + expect(policy.removePreset).not.toHaveBeenCalled(); + expect(stopSandboxMock).toHaveBeenCalledOnce(); + expect(stopSandboxMock).toHaveBeenCalledWith("alpha"); + }); + it("channels start teams re-establishes the MSTEAMS_PORT host forward after rebuild-now completes", async () => { arrangeRegistry({ current: makeTeamsEntry("alpha", { disabled: true, port: "3978" }) }); getDisabledChannelsMock.mockReturnValue(["teams"]); @@ -1256,6 +1267,8 @@ describe("Teams host-forward lifecycle (PRA-2)", () => { expect(ensureMessagingHostForwardAfterRebuildMock).toHaveBeenCalledWith( "alpha", expect.any(Object), + undefined, + expect.any(Function), ); expect(ensureMessagingHostForwardAfterRebuildMock.mock.invocationCallOrder[0]).toBeGreaterThan( rebuildSandboxMock.mock.invocationCallOrder[0], @@ -1267,6 +1280,63 @@ describe("Teams host-forward lifecycle (PRA-2)", () => { }); }); + it("channels start restores the disabled plan when policy authority changes before rebuild (#9833)", async () => { + const current = makeTeamsEntry("alpha", { disabled: true, port: "3978" }); + const priorMessaging = structuredClone(current.messaging); + arrangeRegistry({ current }); + getDisabledChannelsMock.mockReturnValue(["teams"]); + updateSandboxMock.mockImplementation((_name: string, updates: Partial) => { + Object.assign(current, updates); + return true; + }); + const refusal = new PolicyAuthorityRefusalError( + "OpenShell policy authority changed after the channel plan was enabled", + ); + vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority).mockImplementation(() => { + return updateSandboxMock.mock.calls.length > 0 + ? (() => { + throw refusal; + })() + : "nemoclaw-managed"; + }); + + await expect(startSandboxChannel("alpha", { channel: "teams" })).rejects.toBe(refusal); + + expect(current.messaging).toEqual(priorMessaging); + expect(rebuildSandboxMock).not.toHaveBeenCalled(); + expect(ensureMessagingHostForwardAfterRebuildMock).not.toHaveBeenCalled(); + expect(stopSandboxMock).not.toHaveBeenCalled(); + }); + + it("channels start stops the rebuilt sandbox when policy authority changes afterward (#9833)", async () => { + const current = makeTeamsEntry("alpha", { disabled: true, port: "3978" }); + const priorMessaging = structuredClone(current.messaging); + arrangeRegistry({ current }); + getDisabledChannelsMock.mockReturnValue(["teams"]); + updateSandboxMock.mockImplementation((_name: string, updates: Partial) => { + Object.assign(current, updates); + return true; + }); + const refusal = new PolicyAuthorityRefusalError( + "OpenShell policy authority changed after the sandbox rebuild", + ); + vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority).mockImplementation(() => { + return rebuildSandboxMock.mock.calls.length > 0 + ? (() => { + throw refusal; + })() + : "nemoclaw-managed"; + }); + + await expect(startSandboxChannel("alpha", { channel: "teams" })).rejects.toBe(refusal); + + expect(current.messaging).toEqual(priorMessaging); + expect(rebuildSandboxMock).toHaveBeenCalledOnce(); + expect(ensureMessagingHostForwardAfterRebuildMock).not.toHaveBeenCalled(); + expect(stopSandboxMock).toHaveBeenCalledOnce(); + expect(stopSandboxMock).toHaveBeenCalledWith("alpha"); + }); + it("rebuilds before a credential-bound Hermes Discord policy reaches the replacement sandbox", async () => { const current = makeHermesDiscordEntry("alpha"); arrangeRegistry({ current }); diff --git a/src/lib/actions/sandbox/policy-channel-dependencies.ts b/src/lib/actions/sandbox/policy-channel-dependencies.ts index a8cafb39dbe..578d39a3d71 100644 --- a/src/lib/actions/sandbox/policy-channel-dependencies.ts +++ b/src/lib/actions/sandbox/policy-channel-dependencies.ts @@ -2,7 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 import { inspectOpenShellSandboxIdentityFingerprint } from "../../adapters/openshell/policy-authority"; -import { runOpenshell } from "../../adapters/openshell/runtime"; +import { runOpenshell as defaultRunOpenshell } from "../../adapters/openshell/runtime"; +import { + isPolicyAuthorityRefusalError, + preflightSandboxPolicyAuthority, +} from "./policy-authority/preflight"; type MessagingProviderTokenDefinition = { name: string; @@ -14,6 +18,7 @@ type MessagingProviderTokenDefinition = { type MessagingProviderUpsertOptions = { replaceExisting?: boolean; bestEffort?: boolean; + revalidatePolicyRequirements?(operation: string): void; requireExactBindings?: boolean; }; @@ -28,12 +33,13 @@ type LegacyOnboardProvidersModule = { }; upsertMessagingProviders( tokenDefs: MessagingProviderTokenDefinition[], - run: typeof runOpenshell, + run: typeof defaultRunOpenshell, options?: MessagingProviderUpsertOptions, ): string[]; }; type RebuildModule = typeof import("./rebuild"); +type StopModule = typeof import("./stop"); type SetupInferenceModule = typeof import("../../onboard/setup-inference"); type SandboxProviderCleanupModule = typeof import("../../onboard/sandbox-provider-cleanup"); type PolicyModule = typeof import("../../policy"); @@ -50,10 +56,10 @@ type GooglechatWebhookProxy = Pick< "readGooglechatWebhookProxyState" | "startGooglechatWebhookProxy" | "stopGooglechatWebhookProxy" >; -function gatewayRunner(gatewayName: string): typeof runOpenshell { +function gatewayRunner(gatewayName: string): typeof defaultRunOpenshell { const { createGatewayScopedOpenshellRunner } = require("../../onboard/setup-inference") as SetupInferenceModule; - return createGatewayScopedOpenshellRunner(runOpenshell, gatewayName); + return createGatewayScopedOpenshellRunner(defaultRunOpenshell, gatewayName); } /** @@ -64,6 +70,9 @@ function gatewayRunner(gatewayName: string): typeof runOpenshell { * onboarding and rebuild modules at policy-channel import time. */ export const policyChannelDependencies = { + isPolicyAuthorityRefusalError, + preflightSandboxPolicyAuthority, + runOpenshell: (...args: Parameters) => defaultRunOpenshell(...args), deleteMessagingProviderWithRecovery( providerName: string, sandboxName: string, @@ -85,9 +94,9 @@ export const policyChannelDependencies = { }, runGatewayOpenshell( gatewayName: string, - args: Parameters[0], - options?: Parameters[1], - ): ReturnType { + args: Parameters[0], + options?: Parameters[1], + ): ReturnType { return gatewayRunner(gatewayName)(args, options); }, inspectMessagingProviderAttachmentTarget(sandboxName: string, gatewayName: string): string { @@ -125,6 +134,12 @@ export const policyChannelDependencies = { const rebuild = require("./rebuild") as RebuildModule; return rebuild.rebuildSandbox(sandboxName, args); }, + stopSandbox( + sandboxName: Parameters[0], + ): ReturnType { + const stop = require("./stop") as StopModule; + return stop.stopSandbox(sandboxName); + }, stopGooglechatWebhookTunnel(sandboxName: string): void { const lifecycle = require("../../messaging/channels/googlechat/tunnel/lifecycle") as GooglechatWebhookLifecycleModule; diff --git a/src/lib/actions/sandbox/policy-channel-refresh.test.ts b/src/lib/actions/sandbox/policy-channel-refresh.test.ts index 396a0e20194..c16f5dbd2db 100644 --- a/src/lib/actions/sandbox/policy-channel-refresh.test.ts +++ b/src/lib/actions/sandbox/policy-channel-refresh.test.ts @@ -52,6 +52,7 @@ let applyPresetMock: MockInstance; let removePresetMock: MockInstance; let applyPresetContentMock: MockInstance; let loadPresetFromFileMock: MockInstance; +let updateSessionMock: MockInstance; async function captureExit(action: () => Promise): Promise { try { @@ -85,9 +86,9 @@ beforeEach(() => { vi.spyOn(registry, "getCustomPolicies").mockReturnValue([]); vi.spyOn(onboardSession, "loadSession").mockReturnValue(null); - vi.spyOn(onboardSession, "updateSession").mockReturnValue( - undefined as unknown as onboardSession.Session, - ); + updateSessionMock = vi + .spyOn(onboardSession, "updateSession") + .mockReturnValue(undefined as unknown as onboardSession.Session); vi.spyOn(policies, "listPresets").mockReturnValue(POLICY_PRESETS); vi.spyOn(policies, "listCustomPresets").mockReturnValue([]); @@ -165,6 +166,22 @@ describe("addSandboxPolicy refresh contract", () => { expect(refreshSpy).not.toHaveBeenCalled(); }); + it("does not attribute an externally managed built-in policy (#9833)", async () => { + vi.mocked(registry.getSandbox).mockReturnValue({ + name: "alpha", + agent: null, + policies: [], + policyAuthority: "externally-managed", + }); + + await addSandboxPolicy("alpha", { preset: "pypi", yes: true }); + + expect(applyPresetMock).toHaveBeenCalledOnce(); + expect(updateSessionMock).not.toHaveBeenCalled(); + expect(refreshSpy).not.toHaveBeenCalled(); + expect(registry.getSandbox("alpha")?.policies).toEqual([]); + }); + it("does not refresh when the preset name is unknown", async () => { await expect( captureExit(() => addSandboxPolicy("alpha", { preset: "nonexistent", yes: true })), @@ -209,6 +226,23 @@ describe("applyExternalPreset refresh contract (--from-file)", () => { expect(refreshSpy).not.toHaveBeenCalled(); }); + it("does not attribute an externally managed custom policy (#9833)", async () => { + vi.mocked(registry.getSandbox).mockReturnValue({ + name: "alpha", + agent: null, + policies: [], + customPolicies: [], + policyAuthority: "externally-managed", + }); + + await addSandboxPolicy("alpha", { fromFile: tempFile, yes: true }); + + expect(applyPresetContentMock).toHaveBeenCalledOnce(); + expect(updateSessionMock).not.toHaveBeenCalled(); + expect(refreshSpy).not.toHaveBeenCalled(); + expect(registry.getSandbox("alpha")?.customPolicies).toEqual([]); + }); + it("does not refresh on --dry-run with --from-file", async () => { await addSandboxPolicy("alpha", { fromFile: tempFile, yes: true, dryRun: true }); diff --git a/src/lib/actions/sandbox/policy-channel-remove-flow.test.ts b/src/lib/actions/sandbox/policy-channel-remove-flow.test.ts index b1b0a71030e..4ffb667ca93 100644 --- a/src/lib/actions/sandbox/policy-channel-remove-flow.test.ts +++ b/src/lib/actions/sandbox/policy-channel-remove-flow.test.ts @@ -3,8 +3,11 @@ import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from "vitest"; +import { PolicyAuthorityRefusalError } from "../../adapters/openshell/policy-authority"; import * as openshellRuntime from "../../adapters/openshell/runtime"; import * as defs from "../../agent/defs"; +import * as credentials from "../../credentials/store"; +import * as gatewayRuntimeAction from "../../gateway-runtime-action"; import { createBuiltInChannelManifestRegistry, createBuiltInMessagingHookRegistry, @@ -28,6 +31,9 @@ describe("policy channel remove/enable flows", () => { throw new Error(`process.exit(${code ?? 0})`); }) as never); logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); + vi.spyOn(policyChannelDependencies, "preflightSandboxPolicyAuthority").mockReturnValue( + "nemoclaw-managed", + ); vi.spyOn(console, "error").mockImplementation(() => undefined); vi.spyOn(processRecovery, "executeSandboxExecCommand").mockReturnValue({ status: 0, @@ -38,6 +44,7 @@ describe("policy channel remove/enable flows", () => { }); afterEach(() => { + vi.unstubAllEnvs(); vi.restoreAllMocks(); }); @@ -84,14 +91,65 @@ describe("policy channel remove/enable flows", () => { return { rebuildSandbox, removePreset, updateSandbox }; } + async function arrangeOpenClawSlackRemoval() { + const plan = await new MessagingWorkflowPlanner( + createBuiltInChannelManifestRegistry(), + createBuiltInMessagingHookRegistry(), + createBuiltInRenderTemplateResolver(), + ).buildPlan({ + sandboxName: "alpha", + agent: "openclaw", + workflow: "onboard", + isInteractive: false, + configuredChannels: ["slack"], + credentialAvailability: { + SLACK_APP_TOKEN: true, + SLACK_BOT_TOKEN: true, + }, + }); + const current = { + name: "alpha", + agent: "openclaw", + policyAuthority: "externally-managed", + messaging: { schemaVersion: 1, plan }, + } as SandboxEntry; + vi.spyOn(defs, "loadAgent").mockReturnValue({ + name: "openclaw", + displayName: "OpenClaw", + configPaths: { dir: "/sandbox/.openclaw" }, + stateDirs: [], + } as unknown as defs.AgentDefinition); + vi.spyOn(registry, "getSandbox").mockReturnValue(current); + vi.spyOn(registry, "getConfiguredMessagingChannelsFromEntry").mockReturnValue(["slack"]); + vi.spyOn(registry, "getDisabledChannels").mockReturnValue([]); + const updateSandbox = vi.spyOn(registry, "updateSandbox").mockReturnValue(true); + vi.spyOn(policies, "getAppliedPresets").mockReturnValue(["slack"]); + vi.spyOn(policies, "listPresets").mockReturnValue([{ name: "slack" } as never]); + const removePreset = vi.spyOn(policies, "removePreset").mockImplementation(() => { + throw new PolicyAuthorityRefusalError("Slack policy is externally managed"); + }); + vi.spyOn(gatewayRuntimeAction, "recoverNamedGatewayRuntime").mockResolvedValue({ + recovered: true, + before: { state: "healthy_named" }, + after: { state: "healthy_named" }, + attempted: false, + } as never); + const commands: string[][] = []; + const runOpenshell = vi + .spyOn(policyChannelDependencies, "runOpenshell") + .mockImplementation((args) => { + commands.push([...args]); + return { status: 0, stdout: "", stderr: "" } as never; + }); + return { commands, removePreset, runOpenshell, updateSandbox }; + } + function expectHermesSessionCleanup(command: unknown) { expect(String(command)).toContain("/sandbox/.hermes/platforms/whatsapp"); expect(String(command)).toContain( "/sandbox/.hermes/profiles/dashboard-home/platforms/whatsapp/session", ); - expect(String(command)).toContain( - "/sandbox/.hermes/dashboard-home/platforms/whatsapp/session", - ); + expect(String(command)).toContain("/sandbox/.hermes/dashboard-home/platforms/whatsapp/session"); } async function removeWhatsappNonInteractive() { @@ -147,10 +205,96 @@ describe("policy channel remove/enable flows", () => { ).toBeLessThan(updateSandbox.mock.invocationCallOrder[0]); }); + it("does not report tokenless removal before the messaging plan is persisted (#9833)", async () => { + const { updateSandbox } = await arrangeHermesWhatsappRemoval(); + updateSandbox.mockReturnValue(false); + + await expect(removeWhatsappNonInteractive()).rejects.toThrow("process.exit(1)"); + + expect(logSpy.mock.calls.flat().join("\n")).not.toContain("Removed whatsapp channel."); + }); + + it("continues owned channel cleanup when external policy removal refuses (#9833)", async () => { + const { commands, removePreset, runOpenshell, updateSandbox } = + await arrangeOpenClawSlackRemoval(); + vi.stubEnv("SLACK_BOT_TOKEN", "xoxb-host-only"); + vi.stubEnv("SLACK_APP_TOKEN", "xapp-host-only"); + vi.stubEnv("NEMOCLAW_NON_INTERACTIVE", "1"); + + await expect(removeSandboxChannel("alpha", { channel: "slack" })).resolves.toBeUndefined(); + + expect(credentials.getCredential("SLACK_BOT_TOKEN")).toBeNull(); + expect(credentials.getCredential("SLACK_APP_TOKEN")).toBeNull(); + expect( + commands.filter( + (args) => args[0] === "sandbox" && args[1] === "provider" && args[2] === "detach", + ), + ).toHaveLength(2); + expect(commands.filter((args) => args[0] === "provider" && args[1] === "delete")).toHaveLength( + 2, + ); + const firstDetach = commands.findIndex( + (args) => args[0] === "sandbox" && args[1] === "provider" && args[2] === "detach", + ); + expect(firstDetach).toBeGreaterThanOrEqual(0); + expect(runOpenshell.mock.invocationCallOrder[firstDetach]).toBeLessThan( + removePreset.mock.invocationCallOrder[0], + ); + expect(removePreset).toHaveBeenCalledWith("alpha", "slack"); + expect(updateSandbox).toHaveBeenCalled(); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it("stops an owned Google Chat tunnel when external policy removal refuses (#9833)", async () => { + vi.spyOn(defs, "loadAgent").mockReturnValue({ + name: "openclaw", + displayName: "OpenClaw", + configPaths: { dir: "/sandbox/.openclaw" }, + stateDirs: [], + } as unknown as defs.AgentDefinition); + vi.spyOn(registry, "getSandbox").mockReturnValue({ + name: "alpha", + agent: "openclaw", + policyAuthority: "externally-managed", + }); + vi.spyOn(registry, "getConfiguredMessagingChannelsFromEntry").mockReturnValue([]); + vi.spyOn(policies, "getAppliedPresets").mockReturnValue(["googlechat"]); + vi.spyOn(policies, "listPresets").mockReturnValue([{ name: "googlechat" } as never]); + const removePreset = vi.spyOn(policies, "removePreset").mockImplementation(() => { + throw new PolicyAuthorityRefusalError("Google Chat policy is externally managed"); + }); + vi.spyOn(gatewayRuntimeAction, "recoverNamedGatewayRuntime").mockResolvedValue({ + recovered: true, + before: { state: "healthy_named" }, + after: { state: "healthy_named" }, + attempted: false, + } as never); + vi.spyOn(policyChannelDependencies, "runOpenshell").mockReturnValue({ + status: 0, + stdout: "", + stderr: "", + } as never); + const stopTunnel = vi + .spyOn(policyChannelDependencies, "stopGooglechatWebhookTunnel") + .mockImplementation(() => undefined); + vi.stubEnv("NEMOCLAW_NON_INTERACTIVE", "1"); + + await expect(removeSandboxChannel("alpha", { channel: "googlechat" })).resolves.toBeUndefined(); + + expect(stopTunnel).toHaveBeenCalledWith("alpha"); + expect(removePreset).toHaveBeenCalledWith("alpha", "googlechat"); + expect(stopTunnel.mock.invocationCallOrder[0]).toBeLessThan( + removePreset.mock.invocationCallOrder[0], + ); + expect(exitSpy).not.toHaveBeenCalled(); + }); + it.each([ { scenario: "exec transport", execStatus: 0, usesSsh: false }, { scenario: "SSH fallback", execStatus: 1, usesSsh: true }, - ])("clears every Hermes WhatsApp session path through $scenario", async ({ execStatus, usesSsh }) => { + ])( + "clears every Hermes WhatsApp session path through $scenario", + async ({ execStatus, usesSsh }) => { const { updateSandbox } = await arrangeHermesWhatsappRemoval(); vi.mocked(processRecovery.executeSandboxExecCommand).mockReturnValue({ status: execStatus, @@ -180,36 +324,35 @@ describe("policy channel remove/enable flows", () => { expect(transport.mock.invocationCallOrder[0]).toBeLessThan( updateSandbox.mock.invocationCallOrder[0], ); - }); + }, + ); it("keeps channel state unchanged when both Hermes cleanup transports fail", async () => { - const { rebuildSandbox, removePreset, updateSandbox } = await arrangeHermesWhatsappRemoval(); - const runOpenshell = vi.spyOn(openshellRuntime, "runOpenshell"); - vi.mocked(processRecovery.executeSandboxExecCommand).mockReturnValue({ - status: 1, - stdout: "", - stderr: "exec unavailable", - }); - vi.mocked(processRecovery.executeSandboxCommand).mockReturnValue({ - status: 1, - stdout: "", - stderr: "ssh unavailable", - }); + const { rebuildSandbox, removePreset, updateSandbox } = await arrangeHermesWhatsappRemoval(); + const runOpenshell = vi.spyOn(openshellRuntime, "runOpenshell"); + vi.mocked(processRecovery.executeSandboxExecCommand).mockReturnValue({ + status: 1, + stdout: "", + stderr: "exec unavailable", + }); + vi.mocked(processRecovery.executeSandboxCommand).mockReturnValue({ + status: 1, + stdout: "", + stderr: "ssh unavailable", + }); - await expect(removeWhatsappNonInteractive()).rejects.toThrow("process.exit(1)"); + await expect(removeWhatsappNonInteractive()).rejects.toThrow("process.exit(1)"); - expectHermesSessionCleanup( - vi.mocked(processRecovery.executeSandboxExecCommand).mock.calls[0]?.[1], - ); - expectHermesSessionCleanup( - vi.mocked(processRecovery.executeSandboxCommand).mock.calls[0]?.[1], - ); + expectHermesSessionCleanup( + vi.mocked(processRecovery.executeSandboxExecCommand).mock.calls[0]?.[1], + ); + expectHermesSessionCleanup(vi.mocked(processRecovery.executeSandboxCommand).mock.calls[0]?.[1]); - expect(runOpenshell).not.toHaveBeenCalled(); - expect(updateSandbox).not.toHaveBeenCalled(); - expect(removePreset).not.toHaveBeenCalled(); - expect(rebuildSandbox).not.toHaveBeenCalled(); - }); + expect(runOpenshell).not.toHaveBeenCalled(); + expect(updateSandbox).not.toHaveBeenCalled(); + expect(removePreset).not.toHaveBeenCalled(); + expect(rebuildSandbox).not.toHaveBeenCalled(); + }); it("supports stop dry runs for configured Hermes channels", async () => { vi.spyOn(registry, "getSandbox").mockReturnValue({ name: "alpha", agent: "hermes" }); diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index 5edab936a48..b7418ff62fe 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -3,7 +3,6 @@ import fs from "node:fs"; import path from "node:path"; -import { runOpenshell } from "../../adapters/openshell/runtime"; import { type AgentDefinition, loadAgent } from "../../agent/defs"; import { CLI_DISPLAY_NAME, CLI_NAME } from "../../cli/branding"; import { isNonInteractiveEnv, isNonInteractiveSession } from "../../core/non-interactive"; @@ -77,6 +76,7 @@ import { hashCredential } from "../../security/credential-hash"; import { withSandboxMutationLock } from "../../state/mcp-lifecycle-lock"; import * as onboardSession from "../../state/onboard-session"; import * as registry from "../../state/registry"; +import type { SandboxEntry } from "../../state/registry/types"; import { isDockerRuntimeDown, printDockerRuntimeDownGuidance } from "./gateway-failure-classifier"; import { getSandboxTargetGatewayName } from "./gateway-target"; import { ensureMessagingHostForwardAfterRebuild } from "./messaging-host-forward-lifecycle"; @@ -85,6 +85,8 @@ import { refreshSandboxPolicyContextFile } from "./policy-context-refresh"; import { executeSandboxCommand, executeSandboxExecCommand } from "./process-recovery"; const isNonInteractive = () => isNonInteractiveSession(); +const runOpenshell = (...args: Parameters) => + policyChannelDependencies.runOpenshell(...args); /** * Report that `NEMOCLAW_NON_INTERACTIVE=1` leaves no interactive picker, and @@ -138,6 +140,18 @@ type ChannelMutationOptions = { force?: boolean; }; +type ChannelAddRollbackSnapshot = { + readonly presetState: Exclude; + readonly staticProviderType: string | null; + readonly registryPresetWasAttributed: boolean; + readonly priorCreds: Readonly>; + readonly priorMessaging: SandboxEntry["messaging"]; + readonly sessionPresetWasAttributed: boolean | "not-applicable" | "uninspectable"; + readonly wasAlreadyEnabled: boolean; +}; + +type ChannelPresetApplicationState = "not-started" | "ambiguous" | "applied"; + function withSandboxMutationLockUnlessPreview( sandboxName: string, dryRun: boolean | undefined, @@ -430,8 +444,7 @@ async function addSandboxPolicyUnlocked( if (!policies.applyPreset(sandboxName, answer, { suppressDisclosure: true })) { process.exit(1); } - syncSessionPolicyPresetsWithRegistry(sandboxName, answer, "add"); - refreshSandboxPolicyContextFile(sandboxName); + syncManagedPolicyAttribution(sandboxName, answer); } /** @@ -508,10 +521,7 @@ async function applyExternalPreset( suppressDisclosure: true, }); if (result !== false) { - // Custom presets share the registry slot with built-ins (customPolicies - // in policy/index.ts:684), so they need the same session-sync. - syncSessionPolicyPresetsWithRegistry(sandboxName, loaded.presetName, "add"); - refreshSandboxPolicyContextFile(sandboxName); + syncManagedPolicyAttribution(sandboxName, loaded.presetName); } return result !== false; } catch (err: unknown) { @@ -848,8 +858,9 @@ async function applyChannelAddToGatewayAndRegistry( sandboxName: string, channelName: string, acquired: Record, - applyPolicyAfterAttachment?: () => boolean, -): Promise { + prepareMutation?: () => void, + rollbackTokens: Readonly> = {}, +): Promise { const sandboxAgent = registry.getSandbox(sandboxName)?.agent; const staticProviderType = staticMessagingProviderTypeForChannel(channelName, sandboxAgent); const tokenDefs: MessagingTokenDef[] = Object.entries(acquired).map(([envKey, token]) => ({ @@ -900,21 +911,26 @@ async function applyChannelAddToGatewayAndRegistry( console.error(` ${gatewayStartGuidance(gatewayName)}`); process.exit(1); } - policyChannelDependencies.revalidateChannelProviderPolicyAuthority(sandboxName, gatewayName); + try { + prepareMutation?.(); + } catch (error) { + if (policyChannelDependencies.isPolicyAuthorityRefusalError(error)) throw error; + console.error(` ${error instanceof Error ? error.message : String(error)}`); + process.exit(1); + } try { // bestEffort: failures throw (instead of process.exit inside the helper) // so a partial add can be torn down below before exiting. - const providerNames = policyChannelDependencies.upsertMessagingProviders( - tokenDefs, - gatewayName, - { - bestEffort: true, - requireExactBindings: true, - }, - ); + const providerNames = policyChannelDependencies.upsertMessagingProviders(tokenDefs, gatewayName, { + bestEffort: true, + revalidatePolicyRequirements: () => prepareMutation?.(), + requireExactBindings: true, + }); for (const providerName of providerNames) { + prepareMutation?.(); revalidateMessagingProviderAttachmentTarget(sandboxName, gatewayName); - const attached = runOpenshell( + const attached = policyChannelDependencies.runGatewayOpenshell( + gatewayName, ["sandbox", "provider", "attach", "-g", gatewayName, sandboxName, providerName], { ignoreError: true, stdio: ["ignore", "pipe", "pipe"] }, ); @@ -923,104 +939,57 @@ async function applyChannelAddToGatewayAndRegistry( `OpenShell did not attach messaging provider '${providerName}' to sandbox '${sandboxName}'.`, ); } + prepareMutation?.(); revalidateMessagingProviderAttachmentTarget(sandboxName, gatewayName); } - if (applyPolicyAfterAttachment && !applyPolicyAfterAttachment()) { - return null; - } } catch (err) { console.error( ` ✗ Failed to register '${channelName}' providers with the gateway: ${ err instanceof Error ? err.message : String(err) }`, ); - if ( - policyChannelDependencies.isMessagingProviderBindingConflict(err) || - policyChannelDependencies.isMessagingProviderMutationFailure(err) - ) { - const createdProviderNames = [...(err.createdProviderNames ?? [])]; - const createdProviders = new Set(createdProviderNames); - const cleanupFailures = createdProviderNames.filter((providerName) => { - const result = policyChannelDependencies.runGatewayOpenshell( - gatewayName, - ["provider", "delete", providerName], - { ignoreError: true, stdio: ["ignore", "pipe", "pipe"] }, - ); - const output = `${result.stdout || ""}${result.stderr || ""}`; - return result.status !== 0 && !/\bNotFound\b|not found/i.test(output); - }); - const updatedProviderNames = err.mutatedProviderNames.filter( - (providerName) => !createdProviders.has(providerName), - ); - if (updatedProviderNames.length > 0) { - console.error( - ` ${YW}⚠${R} Updated provider state remains for ${updatedProviderNames.join(", ")}; resolve the conflicting provider, then rerun '${CLI_NAME} ${sandboxName} channels add ${channelName}'.`, - ); - } - if (cleanupFailures.length > 0) { + const channel = getChannelDef(channelName); + if (channel) clearChannelTokens(channel); + persistChannelTokens({ ...rollbackTokens }); + if (policyChannelDependencies.isMessagingProviderBindingConflict(err)) { + if (err.mutatedProviderNames.length > 0) { console.error( - ` ${YW}⚠${R} Could not remove newly created providers ${cleanupFailures.join(", ")}; rerun '${CLI_NAME} ${sandboxName} channels remove ${channelName}'.`, + ` ${YW}⚠${R} Provider state changed before the identity conflict; inspect ${err.mutatedProviderNames.join(", ")} before retrying.`, ); } + // Exact-binding preflight does not mutate an incompatible provider. Do + // not run generic teardown here: it could delete that existing provider. process.exit(1); } const teardown = await applyChannelRemoveToGatewayAndRegistry( sandboxName, channelName, Object.keys(acquired), - { bestEffort: true }, + { + bestEffort: true, + }, ); if (!teardown.ok) { console.error( ` ${YW}⚠${R} Partial provider state may remain; run '${CLI_NAME} ${sandboxName} channels remove ${channelName}' once the gateway is reachable.`, ); } + if (policyChannelDependencies.isPolicyAuthorityRefusalError(err)) throw err; process.exit(1); } return true; } -export function revalidateMessagingProviderAttachmentTarget( - sandboxName: string, - gatewayName: string, -): void { - const expected = registry.getSandbox(sandboxName); - const lifecycleGeneration = expected?.lifecycleGeneration; - const expectedFingerprint = expected?.lifecycleLiveIdentityFingerprint; - if ( - !expected || - typeof lifecycleGeneration !== "string" || - typeof expectedFingerprint !== "string" || - (expected.gatewayName && expected.gatewayName !== gatewayName) - ) { - throw new Error( - `Sandbox '${sandboxName}' has incomplete lifecycle identity for messaging provider attachment.`, - ); - } - const liveFingerprint = policyChannelDependencies.inspectMessagingProviderAttachmentTarget( - sandboxName, - gatewayName, - ); - const confirmed = registry.getSandbox(sandboxName); - if ( - liveFingerprint !== expectedFingerprint || - confirmed?.lifecycleGeneration !== lifecycleGeneration || - confirmed.lifecycleLiveIdentityFingerprint !== expectedFingerprint || - (confirmed.gatewayName && confirmed.gatewayName !== gatewayName) - ) { - throw new Error( - `Sandbox '${sandboxName}' changed before messaging provider attachment completed.`, - ); - } -} - // Remove a channel's bridge providers from the gateway and drop it from the // compiled messaging plan. Mirrors applyChannelAddToGatewayAndRegistry. async function applyChannelRemoveToGatewayAndRegistry( sandboxName: string, channelName: string, channelTokenKeys: string[], - options: { bestEffort?: boolean } = {}, + options: { + bestEffort?: boolean; + prepareCleanup?: () => void; + } = {}, ): Promise<{ ok: boolean; residual: string[] }> { const bestEffort = Boolean(options.bestEffort); const residual: string[] = []; @@ -1036,9 +1005,9 @@ async function applyChannelRemoveToGatewayAndRegistry( ...bridgeProviderNamesForChannel(sandboxName, channelName), ]), ]; - const gatewayName = getSandboxTargetGatewayName(sandboxName); if (providerNames.length > 0) { + const gatewayName = getSandboxTargetGatewayName(sandboxName); const recovery = await recoverNamedGatewayRuntime({ gatewayName }); if (!recovery.recovered) { console.error( @@ -1060,6 +1029,7 @@ async function applyChannelRemoveToGatewayAndRegistry( // previous run may have already detached, or the channel may have been // configured for a sandbox that is no longer alive. const detachFailures: Array<{ name: string; output: string }> = []; + options.prepareCleanup?.(); if (gatewayReachable) { for (const name of providerNames) { const result = policyChannelDependencies.runGatewayOpenshell( @@ -1102,14 +1072,13 @@ async function applyChannelRemoveToGatewayAndRegistry( if (gatewayReachable) { const detachFailedSet = new Set(detachFailures.map((f) => f.name)); for (const name of providerNames) { - if (detachFailedSet.has(name)) continue; - const result = policyChannelDependencies.deleteMessagingProviderWithRecovery( - name, - sandboxName, - gatewayName, - ); - if (!result.ok) { - const output = `${result.stdout}${result.stderr}`; + if (!bestEffort && detachFailedSet.has(name)) continue; + const result = runOpenshell(["provider", "delete", name], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.status !== 0) { + const output = `${result.stdout || ""}${result.stderr || ""}`; if (!/\bNotFound\b|not found/i.test(output)) { deleteFailures.push({ name, output: output.trim() }); } @@ -1133,7 +1102,11 @@ async function applyChannelRemoveToGatewayAndRegistry( return { ok: residual.length === 0, residual }; } -async function promptAndRebuild(sandboxName: string, actionDesc: string): Promise { +async function promptAndRebuild( + sandboxName: string, + actionDesc: string, + prepareRebuild?: () => void | Promise, +): Promise { if (isNonInteractive()) { console.log(""); console.log( @@ -1150,10 +1123,28 @@ async function promptAndRebuild(sandboxName: string, actionDesc: string): Promis ); return false; } + await prepareRebuild?.(); await policyChannelDependencies.rebuildSandbox(sandboxName, ["--yes"]); return true; } +function stopSandboxAfterChannelAuthorityRefusal(sandboxName: string, channelName: string): void { + try { + const result = policyChannelDependencies.stopSandbox(sandboxName); + if (result.exitCode !== 0) { + console.error( + ` ${YW}⚠${R} Could not stop sandbox '${sandboxName}' after policy authority changed; '${channelName}' may still be active.`, + ); + } + } catch (error) { + console.error( + ` ${YW}⚠${R} Could not stop sandbox '${sandboxName}' after policy authority changed; '${channelName}' may still be active: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } +} + // Run manifest-owned post-rebuild health hooks. // Failures remain best-effort warnings because the rebuild has already // succeeded; this phase surfaces likely channel startup issues without making @@ -1161,12 +1152,14 @@ async function promptAndRebuild(sandboxName: string, actionDesc: string): Promis async function runMessagingHealthChecksAfterRebuild( sandboxName: string, plan: SandboxMessagingPlan, + validatePolicyAuthority: () => void, ): Promise { if (MessagingSetupApplier.listHealthChecks(plan).length === 0) return; const hookRegistry = createBuiltInMessagingHookRegistry({ openclawBridgeHealth: { sandboxName, + beforeSuccess: validatePolicyAuthority, executeSandboxCommand: (command, timeoutMs) => executeSandboxExecCommand(sandboxName, command, timeoutMs), }, @@ -1192,6 +1185,7 @@ async function runMessagingHealthChecksAfterRebuild( ), }); } catch (err) { + if (policyChannelDependencies.isPolicyAuthorityRefusalError(err)) throw err; console.log(` ${YW}⚠${R} Messaging health check failed: ${formatErrorMessage(err)}`); } } @@ -1205,6 +1199,7 @@ async function planSandboxChannelAdd( channelId: string, agent: AgentDefinition, dependencies: AddSandboxChannelDependencies, + prepareCredentialPersistence: () => void, ): Promise { const planner = new MessagingWorkflowPlanner( messagingManifestRegistry, @@ -1215,6 +1210,11 @@ async function planSandboxChannelAdd( }, tunnelRuntime: policyChannelDependencies.googlechatTunnelRuntime(sandboxName), }, + wechat: { + ilinkLogin: { + beforePersist: prepareCredentialPersistence, + }, + }, }), createBuiltInRenderTemplateResolver(), ); @@ -1246,6 +1246,7 @@ export async function persistManifestChannelDisabledPlan( sandboxName: string, channelId: string, disabled: boolean, + beforePersist?: () => void, ): Promise { const entry = registry.getSandbox(sandboxName); if (!entry?.messaging?.plan) return null; @@ -1268,12 +1269,14 @@ export async function persistManifestChannelDisabledPlan( ? await planner.buildChannelStopPlanFromSandboxEntry(context) : await planner.buildChannelStartPlanFromSandboxEntry(context); if (!plan) return null; + beforePersist?.(); return MessagingHostStateApplier.applyPlanToRegistry(sandboxName, plan) ? plan : null; } export async function persistManifestChannelRemovePlan( sandboxName: string, channelId: string, + beforePersist?: () => void, ): Promise { const entry = registry.getSandbox(sandboxName); if (!entry) return false; @@ -1281,6 +1284,7 @@ export async function persistManifestChannelRemovePlan( const agentId = tryGetMessagingAgentId(agent, messagingManifestRegistry.list()); if (agentId === null) { if (entry.messaging?.plan) { + beforePersist?.(); return registry.updateSandbox(sandboxName, { messaging: undefined }); } return true; @@ -1298,6 +1302,7 @@ export async function persistManifestChannelRemovePlan( supportedChannelIds: availableManifestChannelsForAgent(agent).map((manifest) => manifest.id), }); if (!plan) return !entry.messaging?.plan; + beforePersist?.(); return MessagingHostStateApplier.applyPlanToRegistry(sandboxName, plan); } @@ -1388,7 +1393,7 @@ function loadValidateAndDiscloseChannelPreset( sandboxName: string, channelName: string, verb: "add" | "start", -): policies.PresetPolicyState | null { +): { content: string; disclosedState: policies.PresetPolicyState | null } { const presetContent = policies.loadPresetForSandbox(sandboxName, channelName); const presetPolicyKeys = presetContent === null ? [] : policies.parsePresetPolicyKeys(presetContent); @@ -1405,7 +1410,61 @@ function loadValidateAndDiscloseChannelPreset( ); process.exit(1); } - return discloseChannelPresetScope(sandboxName, channelName, presetContent); + return { + content: presetContent, + disclosedState: discloseChannelPresetScope(sandboxName, channelName, presetContent), + }; +} + +function refuseUnsafeChannelPresetAdd( + sandboxName: string, + channelName: string, + state: policies.PresetPolicyState | null, +): asserts state is Exclude { + if (state === "drift") { + console.error( + ` Cannot add channel '${channelName}': its live policy entries differ from the reviewed preset.`, + ); + console.error( + ` No channel state was changed. Reconcile the colliding policy entries for '${sandboxName}', then retry.`, + ); + process.exit(1); + } + if (state === null) { + console.error( + ` Cannot add channel '${channelName}': the live policy for '${sandboxName}' could not be inspected.`, + ); + console.error(" No channel state was changed. Restore gateway access, then retry."); + process.exit(1); + } +} + +function revalidateChannelPolicyAuthority( + sandboxName: string, + channelName: string, + operation: "add" | "start", + presetContent: string, +): void { + policyChannelDependencies.preflightSandboxPolicyAuthority({ + externalPolicy: "verify", + operation: `${operation} channel '${channelName}'`, + requiredPolicyContents: [presetContent], + sandboxName, + }); +} + +function preflightChannelPolicyAuthority( + sandboxName: string, + channelName: string, + operation: "add" | "start", + presetContent: string, +): void { + try { + revalidateChannelPolicyAuthority(sandboxName, channelName, operation, presetContent); + } catch (error) { + console.error(` ${error instanceof Error ? error.message : String(error)}`); + process.exit(1); + } } function safeLoadOnboardSession(): ReturnType { @@ -1416,6 +1475,19 @@ function safeLoadOnboardSession(): ReturnType } } +function snapshotSessionPresetAttribution( + sandboxName: string, + presetName: string, +): ChannelAddRollbackSnapshot["sessionPresetWasAttributed"] { + try { + const session = onboardSession.loadSession(); + if (!session || session.sandboxName !== sandboxName) return "not-applicable"; + return session.policyPresets?.includes(presetName) ?? false; + } catch { + return "uninspectable"; + } +} + export async function addSandboxChannel( sandboxName: string, options: ChannelMutationOptions = {}, @@ -1464,14 +1536,78 @@ async function addSandboxChannelUnlocked( // Disclose before credential collection, conflict prompts, or any gateway / // registry mutation. The core apply path rechecks immediately before set. - const disclosedPresetState = loadValidateAndDiscloseChannelPreset(sandboxName, canonical, "add"); + const presetPreflight = loadValidateAndDiscloseChannelPreset(sandboxName, canonical, "add"); if (dryRun) { console.log(` --dry-run: would enable channel '${canonical}' for '${sandboxName}'.`); return; } - const plan = await planSandboxChannelAdd(sandboxName, canonical, agent, dependencies); + refuseUnsafeChannelPresetAdd(sandboxName, canonical, presetPreflight.disclosedState); + preflightChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content); + + const channelDef = getChannelDef(canonical); + if (!channelDef) { + console.error(` Unknown channel '${canonical}'.`); + process.exit(1); + } + const priorEntry = registry.getSandbox(sandboxName); + const priorCreds: Record = {}; + for (const key of getChannelTokenKeys(channelDef)) { + const existing = getCredential(key); + if (existing != null) priorCreds[key] = existing; + } + const rollbackSnapshot: ChannelAddRollbackSnapshot = { + presetState: presetPreflight.disclosedState, + staticProviderType: staticMessagingProviderTypeForChannel(canonical, agent.name), + registryPresetWasAttributed: priorEntry?.policies?.includes(canonical) ?? false, + priorCreds, + priorMessaging: + priorEntry?.messaging === undefined ? undefined : structuredClone(priorEntry.messaging), + sessionPresetWasAttributed: snapshotSessionPresetAttribution(sandboxName, canonical), + wasAlreadyEnabled: registry + .getConfiguredMessagingChannelsFromEntry(priorEntry) + .includes(canonical), + }; + let presetApplicationState: ChannelPresetApplicationState = "not-started"; + let rollbackPromise: ReturnType | undefined; + let rebuiltRuntime = false; + let runtimeContainmentAttempted = false; + const rollbackAdd = (preserveLivePolicy = false) => + (rollbackPromise ??= rollbackChannelAdd( + sandboxName, + channelDef, + canonical, + rollbackSnapshot, + presetApplicationState, + preserveLivePolicy, + )); + const containRebuiltRuntime = (): void => { + if (!rebuiltRuntime || runtimeContainmentAttempted) return; + runtimeContainmentAttempted = true; + stopSandboxAfterChannelAuthorityRefusal(sandboxName, canonical); + }; + const rollbackAuthorityRefusal = async (error: unknown): Promise => { + if (!policyChannelDependencies.isPolicyAuthorityRefusalError(error)) return; + containRebuiltRuntime(); + await rollbackAdd(true); + }; + const revalidateAfterOwnedMutation = async (): Promise => { + try { + revalidateChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content); + } catch (error) { + containRebuiltRuntime(); + await rollbackAdd(true); + if (policyChannelDependencies.isPolicyAuthorityRefusalError(error)) throw error; + console.error(` ${error instanceof Error ? error.message : String(error)}`); + process.exit(1); + } + }; + + const plan = await planSandboxChannelAdd(sandboxName, canonical, agent, dependencies, () => + preflightChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content), + ); + await revalidateAfterOwnedMutation(); const acquired = collectManifestCredentials(manifest); if (!(await checkChannelAddConflict(sandboxName, canonical, acquired, force))) { return; // user aborted; nothing registered or widened @@ -1487,19 +1623,41 @@ async function addSandboxChannelUnlocked( // host-side credential to acquire; register the bridge now and let the // operator complete pairing after rebuild. if (manifest.auth.mode === "in-sandbox-qr") { - if ( - !applyChannelPresetIfAvailable(sandboxName, canonical, "add", { - disclosedPresetState, - }) - ) { + let presetApplied: boolean; + presetApplicationState = "ambiguous"; + try { + presetApplied = applyChannelPresetIfAvailable(sandboxName, canonical, "add", { + disclosedPresetState: presetPreflight.disclosedState, + expectedExistingNetworkPolicyContent: + presetPreflight.disclosedState === "match" ? presetPreflight.content : null, + propagatePolicyAuthorityRefusal: true, + }); + } catch (error) { + await rollbackAdd(policyChannelDependencies.isPolicyAuthorityRefusalError(error)); + throw error; + } + if (!presetApplied) { + await rollbackAdd(); process.exit(1); } - await applyChannelAddToGatewayAndRegistry(sandboxName, canonical, {}); + presetApplicationState = "applied"; + + await revalidateAfterOwnedMutation(); + try { + await applyChannelAddToGatewayAndRegistry(sandboxName, canonical, {}, () => + revalidateChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content), + ); + } catch (error) { + await rollbackAuthorityRefusal(error); + throw error; + } + await revalidateAfterOwnedMutation(); if (!MessagingHostStateApplier.applyPlanToRegistry(sandboxName, plan)) { console.error(` ${YW}⚠${R} Could not persist messaging plan for '${sandboxName}'.`); - removeChannelPresetIfPresent(sandboxName, canonical); + await rollbackAdd(); process.exit(1); } + await revalidateAfterOwnedMutation(); console.log(""); const help = manifest.enrollmentHelp ?? manifest.inputs[0]?.prompt?.help; if (help) console.log(` ${help}`); @@ -1512,66 +1670,130 @@ async function addSandboxChannelUnlocked( for (const line of manifest.enrollmentNotes ?? []) { console.log(` ${line}`); } - const rebuilt = await promptAndRebuild(sandboxName, `add '${canonical}'`); + let rebuilt: boolean; + try { + rebuilt = await promptAndRebuild( + sandboxName, + `add '${canonical}'`, + revalidateAfterOwnedMutation, + ); + } catch (error) { + await rollbackAuthorityRefusal(error); + throw error; + } + rebuiltRuntime = rebuilt; if (rebuilt) { - ensureMessagingHostForwardAfterRebuild(sandboxName, plan); - await runMessagingHealthChecksAfterRebuild(sandboxName, plan); + await revalidateAfterOwnedMutation(); + try { + ensureMessagingHostForwardAfterRebuild(sandboxName, plan, undefined, () => + revalidateChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content), + ); + } catch (error) { + await rollbackAuthorityRefusal(error); + throw error; + } + await revalidateAfterOwnedMutation(); + try { + await runMessagingHealthChecksAfterRebuild(sandboxName, plan, () => + revalidateChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content), + ); + } catch (error) { + await rollbackAuthorityRefusal(error); + throw error; + } + await revalidateAfterOwnedMutation(); } return; } - const channelDef = getChannelDef(canonical); - if (!channelDef) { - console.error(` Unknown channel '${canonical}'.`); - process.exit(1); - } - const priorEntry = registry.getSandbox(sandboxName); - const wasAlreadyEnabled = registry - .getConfiguredMessagingChannelsFromEntry(priorEntry) - .includes(canonical); - const channelTokenKeys = getChannelTokenKeys(channelDef); - const priorCreds: Record = {}; - for (const key of channelTokenKeys) { - const existing = getCredential(key); - if (existing != null) priorCreds[key] = existing; - } - // Register providers before credentials or durable channel state are saved. - // OpenShell requires credential providers to be attached before their policy - // bindings can be applied, so rollback both effects when policy application fails. - const registeredBridge = await applyChannelAddToGatewayAndRegistry( - sandboxName, - canonical, - acquired, - () => - applyChannelPresetIfAvailable(sandboxName, canonical, "add", { - disclosedPresetState, - }), - ); - if (registeredBridge === null) { - await rollbackChannelAdd(sandboxName, channelDef, canonical, { - wasAlreadyEnabled, - priorCreds, - }); - process.exit(1); + const { wasAlreadyEnabled } = rollbackSnapshot; + preflightChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content); + // Register every provider before credentials or durable channel state are + // saved. Exact-binding preflight occurs before mutation; authority is + // revalidated before each gateway command. + let registeredBridge: boolean; + try { + registeredBridge = await applyChannelAddToGatewayAndRegistry( + sandboxName, + canonical, + acquired, + () => + revalidateChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content), + wasAlreadyEnabled ? priorCreds : {}, + ); + } catch (error) { + await rollbackAuthorityRefusal(error); + throw error; } if (registeredBridge) { + await revalidateAfterOwnedMutation(); console.log(` ${G}✓${R} Registered ${canonical} bridge with the OpenShell gateway.`); } + preflightChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content); persistChannelTokens(acquired); + let presetApplied: boolean; + presetApplicationState = "ambiguous"; + try { + presetApplied = applyChannelPresetIfAvailable(sandboxName, canonical, "add", { + disclosedPresetState: presetPreflight.disclosedState, + expectedExistingNetworkPolicyContent: + presetPreflight.disclosedState === "match" ? presetPreflight.content : null, + propagatePolicyAuthorityRefusal: true, + }); + } catch (error) { + await rollbackAdd(policyChannelDependencies.isPolicyAuthorityRefusalError(error)); + throw error; + } + if (!presetApplied) { + await rollbackAdd(); + process.exit(1); + } + presetApplicationState = "applied"; + + await revalidateAfterOwnedMutation(); if (!MessagingHostStateApplier.applyPlanToRegistry(sandboxName, plan)) { console.error(` ${YW}⚠${R} Could not persist messaging plan for '${sandboxName}'.`); - await rollbackChannelAdd(sandboxName, channelDef, canonical, { - wasAlreadyEnabled, - priorCreds, - }); + console.error( + " Earlier gateway or policy side effects may already have run, but durable channel state was not saved.", + ); + await rollbackAdd(); process.exit(1); } + await revalidateAfterOwnedMutation(); - const rebuilt = await promptAndRebuild(sandboxName, `add '${canonical}'`); + let rebuilt: boolean; + try { + rebuilt = await promptAndRebuild( + sandboxName, + `add '${canonical}'`, + revalidateAfterOwnedMutation, + ); + } catch (error) { + await rollbackAuthorityRefusal(error); + throw error; + } + rebuiltRuntime = rebuilt; if (rebuilt) { - ensureMessagingHostForwardAfterRebuild(sandboxName, plan); - await runMessagingHealthChecksAfterRebuild(sandboxName, plan); + await revalidateAfterOwnedMutation(); + try { + ensureMessagingHostForwardAfterRebuild(sandboxName, plan, undefined, () => + revalidateChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content), + ); + } catch (error) { + await rollbackAuthorityRefusal(error); + throw error; + } + await revalidateAfterOwnedMutation(); + try { + await runMessagingHealthChecksAfterRebuild(sandboxName, plan, () => + revalidateChannelPolicyAuthority(sandboxName, canonical, "add", presetPreflight.content), + ); + } catch (error) { + await rollbackAuthorityRefusal(error); + throw error; + } + await revalidateAfterOwnedMutation(); } } @@ -1579,23 +1801,20 @@ async function rollbackChannelAdd( sandboxName: string, channel: ChannelDef, canonical: string, - snapshot: { - wasAlreadyEnabled: boolean; - priorCreds: Record; - }, + snapshot: ChannelAddRollbackSnapshot, + presetApplicationState: ChannelPresetApplicationState, + preserveLivePolicy: boolean, ): Promise<{ ok: boolean; residual: string[] }> { + let result: { ok: boolean; residual: string[] }; if (snapshot.wasAlreadyEnabled) { console.error( ` ${YW}⚠${R} Restoring prior '${canonical}' configuration; new token rotation aborted.`, ); clearChannelTokens(channel); if (Object.keys(snapshot.priorCreds).length > 0) { - persistChannelTokens(snapshot.priorCreds); + persistChannelTokens({ ...snapshot.priorCreds }); } const residual: string[] = ["gateway-providers"]; - console.error( - ` ${YW}⚠${R} Rollback could not fully clean ${residual.join(", ")}; run '${CLI_NAME} ${sandboxName} channels remove ${canonical}' once the gateway is reachable.`, - ); // The prior bridge secret is env-only and gone — the failed re-add already // overwrote the gateway's refresh material, so a restore is impossible. if (bridgeProviderNamesForChannel(sandboxName, canonical).length > 0) { @@ -1609,13 +1828,14 @@ async function rollbackChannelAdd( name: bridgeProviderName(sandboxName, canonical, envKey), envKey, token, - providerType: MESSAGING_CREDENTIAL_PROVIDER_TYPE, + providerType: snapshot.staticProviderType ?? MESSAGING_CREDENTIAL_PROVIDER_TYPE, })); policyChannelDependencies.upsertMessagingProviders( priorTokenDefs, getSandboxTargetGatewayName(sandboxName), { bestEffort: true, + requireExactBindings: true, }, ); } catch (err) { @@ -1626,38 +1846,186 @@ async function rollbackChannelAdd( ); } } - return { ok: false, residual }; + result = { ok: false, residual }; + } else { + console.error( + ` ${YW}⚠${R} Rolling back '${canonical}' bridge registration to keep messaging plan and policy state aligned.`, + ); + clearChannelTokens(channel); + result = await applyChannelRemoveToGatewayAndRegistry( + sandboxName, + canonical, + getChannelTokenKeys(channel), + { + bestEffort: true, + }, + ); } - console.error( - ` ${YW}⚠${R} Rolling back '${canonical}' bridge registration to keep messaging plan and policy state aligned.`, - ); - clearChannelTokens(channel); - removeChannelPresetIfPresent(sandboxName, canonical); - const result = await applyChannelRemoveToGatewayAndRegistry( + if ( + snapshot.presetState === "absent" && + presetApplicationState === "applied" && + preserveLivePolicy + ) { + result.residual.push("policy-preset"); + result.ok = false; + console.error( + ` ${YW}⚠${R} Policy authority changed; the live '${canonical}' policy was preserved.`, + ); + } else if (snapshot.presetState === "absent" && presetApplicationState === "applied") { + try { + if (!policies.removePreset(sandboxName, canonical, { nonFatal: true })) { + result.residual.push("policy-preset"); + result.ok = false; + } + } catch (error) { + result.residual.push("policy-preset"); + result.ok = false; + console.error( + ` ${YW}⚠${R} Failed to restore the prior live policy for '${canonical}': ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } else if (snapshot.presetState === "absent" && presetApplicationState === "ambiguous") { + result.residual.push("policy-preset"); + result.ok = false; + console.error( + ` ${YW}⚠${R} Could not confirm whether '${canonical}' changed the live policy; no policy cleanup was attempted without proven ownership.`, + ); + } + + const attributionResidual = restoreChannelPresetAttribution( sandboxName, canonical, - getChannelTokenKeys(channel), - { bestEffort: true }, + snapshot, + presetApplicationState, ); + result.residual.push(...attributionResidual); + if (attributionResidual.length > 0) result.ok = false; + try { + if (!registry.updateSandbox(sandboxName, { messaging: snapshot.priorMessaging })) { + result.residual.push("messaging-plan"); + result.ok = false; + } + } catch (error) { + result.residual.push("messaging-plan"); + result.ok = false; + console.error( + ` ${YW}⚠${R} Failed to restore messaging plan for '${canonical}': ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } if (!result.ok) { console.error( - ` ${YW}⚠${R} Rollback could not fully clean ${result.residual.join(", ")}; run '${CLI_NAME} ${sandboxName} channels remove ${canonical}' once the gateway is reachable.`, + ` ${YW}⚠${R} Rollback could not fully clean ${result.residual.join(", ")}; inspect the live policy and local sandbox state before retrying.`, ); } return result; } +function restoreChannelPresetAttribution( + sandboxName: string, + canonical: string, + snapshot: ChannelAddRollbackSnapshot, + presetApplicationState: ChannelPresetApplicationState, +): string[] { + const residual: string[] = []; + let registryAttributionChanged = false; + try { + const entry = registry.getSandbox(sandboxName); + const appliedPresets = entry?.policies ?? []; + const isAttributed = appliedPresets.includes(canonical); + if (isAttributed !== snapshot.registryPresetWasAttributed) { + if (!entry) { + residual.push("policy-registry-attribution"); + } else { + const presetsWithoutChannel = appliedPresets.filter((name) => name !== canonical); + const restoredPresets = snapshot.registryPresetWasAttributed + ? [...presetsWithoutChannel, canonical] + : presetsWithoutChannel; + if (!registry.updateSandbox(sandboxName, { policies: restoredPresets })) { + residual.push("policy-registry-attribution"); + } else { + registryAttributionChanged = true; + } + } + } + } catch (error) { + residual.push("policy-registry-attribution"); + console.error( + ` ${YW}⚠${R} Failed to restore policy attribution for '${canonical}': ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + + if ( + snapshot.sessionPresetWasAttributed === "uninspectable" && + presetApplicationState !== "not-started" + ) { + residual.push("policy-session-attribution"); + console.error( + ` ${YW}⚠${R} Could not restore onboard-session policy attribution for '${canonical}' because its prior state was unreadable.`, + ); + } else if (typeof snapshot.sessionPresetWasAttributed === "boolean") { + try { + const session = onboardSession.loadSession(); + if (!session || session.sandboxName !== sandboxName) { + residual.push("policy-session-attribution"); + } else { + const sessionPresets = Array.isArray(session.policyPresets) ? session.policyPresets : []; + const isAttributed = sessionPresets.includes(canonical); + if (isAttributed !== snapshot.sessionPresetWasAttributed) { + onboardSession.updateSession((current) => { + if (current.sandboxName !== sandboxName) { + throw new Error("onboard session changed during channel rollback"); + } + const currentPresets = Array.isArray(current.policyPresets) + ? current.policyPresets + : []; + const presetsWithoutChannel = currentPresets.filter((name) => name !== canonical); + current.policyPresets = snapshot.sessionPresetWasAttributed + ? [...presetsWithoutChannel, canonical] + : presetsWithoutChannel; + return current; + }); + } + } + } catch (error) { + residual.push("policy-session-attribution"); + console.error( + ` ${YW}⚠${R} Failed to restore onboard-session policy attribution for '${canonical}': ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + + if (registryAttributionChanged) refreshSandboxPolicyContextFile(sandboxName); + return residual; +} + export function applyChannelPresetIfAvailable( sandboxName: string, channelName: string, retryAction: "add" | "start" = "add", - options: { disclosedPresetState?: policies.PresetPolicyState | null } = {}, + options: { + disclosedPresetState?: policies.PresetPolicyState | null; + expectedExistingNetworkPolicyContent?: string | null; + propagatePolicyAuthorityRefusal?: boolean; + } = {}, ): boolean { try { const applied = Object.prototype.hasOwnProperty.call(options, "disclosedPresetState") ? policies.applyPreset(sandboxName, channelName, { disclosedPresetState: options.disclosedPresetState, + ...(Object.prototype.hasOwnProperty.call(options, "expectedExistingNetworkPolicyContent") + ? { + expectedExistingNetworkPolicyContent: options.expectedExistingNetworkPolicyContent, + } + : {}), includeMessagingCredentialBindings: true, }) : policies.applyPreset(sandboxName, channelName, { @@ -1672,10 +2040,14 @@ export function applyChannelPresetIfAvailable( ); return false; } - syncSessionPolicyPresetsWithRegistry(sandboxName, channelName, "add"); - refreshSandboxPolicyContextFile(sandboxName); + syncManagedPolicyAttribution(sandboxName, channelName); return true; } catch (err) { + if ( + options.propagatePolicyAuthorityRefusal && + policyChannelDependencies.isPolicyAuthorityRefusalError(err) + ) + throw err; const msg = err instanceof Error ? err.message : String(err); console.error(` ${YW}⚠${R} Failed to apply '${channelName}' policy preset: ${msg}`); console.error( @@ -1796,40 +2168,48 @@ function syncSessionPolicyPresetsWithRegistry( } } +function syncManagedPolicyAttribution(sandboxName: string, presetName: string): void { + if (registry.getSandbox(sandboxName)?.policyAuthority === "externally-managed") return; + syncSessionPolicyPresetsWithRegistry(sandboxName, presetName, "add"); + refreshSandboxPolicyContextFile(sandboxName); +} + // Mirror of applyChannelPresetIfAvailable. When the channel-named built-in // preset is currently applied to the sandbox, un-apply it so `policy-list` // no longer reports it active and the L7 proxy stops allow-listing the -// channel's upstream API. Removal marks the channel disabled before this -// policy release, so a later provider failure remains retryable. -export function removeChannelPresetIfPresent(sandboxName: string, channelName: string): boolean { +// channel's upstream API (defense-in-depth: bridge is gone, egress to +// api.telegram.org / discord.com / slack.com should follow). Warns but does +// not abort the remove flow — the bridge teardown has already succeeded; +// the operator can run `policy-remove ` manually if cleanup falters. +export function removeChannelPresetIfPresent(sandboxName: string, channelName: string): void { const builtinPresets = new Set(policies.listPresets().map((p) => p.name)); if (!builtinPresets.has(channelName)) { syncSessionPolicyPresetsWithRegistry(sandboxName, channelName, "remove"); - return true; + return; } if (!policies.getAppliedPresets(sandboxName).includes(channelName)) { syncSessionPolicyPresetsWithRegistry(sandboxName, channelName, "remove"); - return true; + return; } try { const removed = policies.removePreset(sandboxName, channelName); if (!removed) { - console.error(` ${YW}⚠${R} Channel '${channelName}' policy preset failed to un-apply.`); + console.error( + ` ${YW}⚠${R} Channel '${channelName}' bridge removed but its policy preset failed to un-apply.`, + ); console.error( ` Run manually after rebuild with: ${CLI_NAME} ${sandboxName} policy remove ${channelName}`, ); - return false; + } else { + syncSessionPolicyPresetsWithRegistry(sandboxName, channelName, "remove"); + refreshSandboxPolicyContextFile(sandboxName); } - syncSessionPolicyPresetsWithRegistry(sandboxName, channelName, "remove"); - refreshSandboxPolicyContextFile(sandboxName); - return true; } catch (err) { const msg = err instanceof Error ? err.message : String(err); console.error(` ${YW}⚠${R} Failed to remove '${channelName}' policy preset: ${msg}`); console.error( ` Run manually after rebuild with: ${CLI_NAME} ${sandboxName} policy remove ${channelName}`, ); - return false; } } @@ -1886,7 +2266,6 @@ async function removeSandboxChannelUnlocked( (registryEntry?.policies || []).includes(canonical) || sessionPolicyPresets.includes(canonical) || policies.getAppliedPresets(sandboxName).includes(canonical); - // The public Google Chat endpoint must stop before credentials, providers, // policy, or durable plan state change. Otherwise a partial teardown leaves // a live webhook with no retryable channel record. Attempt this even when @@ -1913,61 +2292,26 @@ async function removeSandboxChannelUnlocked( // when the registry/policy show no residue — `channels remove` on a // never-configured/already-clean sandbox must remain a quiet no-op even // when the sandbox is stopped (#4001 review). - if ( - isQrChannel && - hasChannelResidue && - !clearSandboxChannelDurableState(sandboxName, canonical) - ) { - console.error( - ` Refusing to proceed: '${canonical}' session state is still inside the sandbox.`, - ); - console.error( - ` Start the sandbox, then re-run: ${CLI_NAME} ${sandboxName} channels remove ${canonical}`, - ); - process.exit(1); - } - - const configuredChannels = registry.getConfiguredMessagingChannelsFromEntry(registryEntry); - if ( - registryEntry?.messaging?.plan && - configuredChannels.includes(canonical) && - !registry.getDisabledChannels(sandboxName).includes(canonical) - ) { - const disabledPlan = await persistManifestChannelDisabledPlan(sandboxName, canonical, true); - if (!disabledPlan) { - console.error(` Could not mark '${canonical}' disabled before removing it.`); + const prepareRemovalCleanup = (): void => { + if ( + isQrChannel && + hasChannelResidue && + !clearSandboxChannelDurableState(sandboxName, canonical) + ) { + console.error( + ` Refusing to proceed: '${canonical}' session state is still inside the sandbox.`, + ); + console.error( + ` Start the sandbox, then re-run: ${CLI_NAME} ${sandboxName} channels remove ${canonical}`, + ); process.exit(1); } - } - - if (!removeChannelPresetIfPresent(sandboxName, canonical)) { - console.error( - ` ${YW}⚠${R} Channel '${canonical}' remains disabled while policy cleanup is incomplete.`, - ); - console.error( - ` Resolve the policy error, then re-run: ${CLI_NAME} ${sandboxName} channels remove ${canonical}`, - ); - process.exit(1); - } - const teardown = await applyChannelRemoveToGatewayAndRegistry(sandboxName, canonical, tokenKeys, { - bestEffort: true, + clearChannelTokens(channel); + }; + await applyChannelRemoveToGatewayAndRegistry(sandboxName, canonical, tokenKeys, { + prepareCleanup: prepareRemovalCleanup, }); - if (!teardown.ok) { - console.error( - ` ${YW}⚠${R} Channel '${canonical}' remains disabled while ${teardown.residual.join(", ")} cleanup is incomplete.`, - ); - console.error( - ` Resolve the gateway error, then re-run: ${CLI_NAME} ${sandboxName} channels remove ${canonical}`, - ); - process.exit(1); - } - clearChannelTokens(channel); - if (tokenKeys.length > 0) { - console.log(` ${G}✓${R} Removed ${canonical} bridge from the OpenShell gateway.`); - } else { - console.log(` ${G}✓${R} Removed ${canonical} channel.`); - } - + removeChannelPresetIfPresent(sandboxName, canonical); if (!(await persistManifestChannelRemovePlan(sandboxName, canonical))) { console.error(` ${YW}⚠${R} Could not persist messaging plan for '${sandboxName}'.`); process.exit(1); @@ -1980,6 +2324,12 @@ async function removeSandboxChannelUnlocked( clearSandboxChannelDurableState(sandboxName, canonical); } + if (tokenKeys.length > 0) { + console.log(` ${G}✓${R} Removed ${canonical} bridge from the OpenShell gateway.`); + } else { + console.log(` ${G}✓${R} Removed ${canonical} channel.`); + } + await promptAndRebuild(sandboxName, `remove '${canonical}'`); } @@ -2039,16 +2389,30 @@ async function sandboxChannelsSetEnabled( return; } - if (!disabled) { - loadValidateAndDiscloseChannelPreset(sandboxName, canonical, "start"); - } + const presetPreflight = disabled + ? undefined + : loadValidateAndDiscloseChannelPreset(sandboxName, canonical, "start"); if (dryRun) { console.log(` --dry-run: would ${verb} channel '${canonical}' for '${sandboxName}'.`); return; } - const plan = await persistManifestChannelDisabledPlan(sandboxName, canonical, disabled); + if (presetPreflight) { + preflightChannelPolicyAuthority(sandboxName, canonical, "start", presetPreflight.content); + } + + const priorMessaging = + registryEntry.messaging === undefined ? undefined : structuredClone(registryEntry.messaging); + const plan = await persistManifestChannelDisabledPlan( + sandboxName, + canonical, + disabled, + presetPreflight + ? () => + preflightChannelPolicyAuthority(sandboxName, canonical, "start", presetPreflight.content) + : undefined, + ); if (!plan) { console.error(` Could not persist messaging plan for '${sandboxName}'.`); process.exit(1); @@ -2058,10 +2422,55 @@ async function sandboxChannelsSetEnabled( // the preset into rebuild, where sandbox creation attaches each provider // before OpenShell accepts its credential-bound policy. const state = disabled ? "disabled" : "enabled"; + let rebuiltRuntime = false; + let startRollbackAttempted = false; + const restoreStartPlan = (): void => { + if (disabled || startRollbackAttempted) return; + startRollbackAttempted = true; + try { + if (!registry.updateSandbox(sandboxName, { messaging: priorMessaging })) { + console.error( + ` ${YW}⚠${R} Could not restore the disabled '${canonical}' plan after start failed.`, + ); + } + } catch (error) { + console.error( + ` ${YW}⚠${R} Could not restore the disabled '${canonical}' plan after start failed: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + }; + const revalidateChannelTransition = (): void => { + if (disabled) return; + try { + revalidateChannelPolicyAuthority(sandboxName, canonical, "start", presetPreflight!.content); + } catch (error) { + restoreStartPlan(); + if (rebuiltRuntime) stopSandboxAfterChannelAuthorityRefusal(sandboxName, canonical); + throw error; + } + }; + revalidateChannelTransition(); console.log(` ${G}✓${R} Marked ${canonical} ${state} for '${sandboxName}'.`); - const rebuilt = await promptAndRebuild(sandboxName, `${verb} '${canonical}'`); + let rebuilt: boolean; + try { + rebuilt = await promptAndRebuild( + sandboxName, + `${verb} '${canonical}'`, + revalidateChannelTransition, + ); + } catch (error) { + restoreStartPlan(); + throw error; + } + rebuiltRuntime = rebuilt; if (rebuilt && !disabled) { - ensureMessagingHostForwardAfterRebuild(sandboxName, plan); + revalidateChannelTransition(); + ensureMessagingHostForwardAfterRebuild(sandboxName, plan, undefined, () => + revalidateChannelTransition(), + ); + revalidateChannelTransition(); } } diff --git a/src/lib/messaging/AGENTS.md b/src/lib/messaging/AGENTS.md index b5e297f68b0..ff4134c8057 100644 --- a/src/lib/messaging/AGENTS.md +++ b/src/lib/messaging/AGENTS.md @@ -45,6 +45,11 @@ The design goal is to keep messaging channel behavior out of core onboard/rebuil - Disabled channels are not active. Always filter effects through `enabledPlanChannels()` or `filterEnabledPlanEntries()` when applying providers, policies, render, hooks, runtime setup, or conflicts. - Conflict detection has two axes: generic credential-hash overlap in `applier/conflict-detection/` and channel-owned `pre-enable` hooks such as Slack Socket Mode gateway checks. - Keep transitional compatibility tables derived from manifests. `src/lib/sandbox/channels.ts` intentionally builds legacy CLI metadata from `listBuiltInMessagingChannelManifests()`. +- Channel add and start must inspect OpenShell policy authority before credential, provider, policy, or registry mutation. +- An externally managed policy must contain each exact channel policy entry before add or start continues. NemoClaw must not record those entries as applied presets. +- Channel remove must clean NemoClaw-managed tunnels, credentials, providers, in-sandbox state, and registry state under either policy authority. +- Channel remove must preserve externally managed policy. Only the policy-removal step may refuse when authority changed or cannot be inspected. +- Add rollback must clean newly written credentials and restore or remove owned providers even when policy verification starts to refuse after the add mutation. Compensation must not change live policy after refusal. ## Adding or Changing a Channel diff --git a/src/lib/messaging/channels/openclaw-bridge-health.ts b/src/lib/messaging/channels/openclaw-bridge-health.ts index 0cd8f1ac455..b1aaec7413d 100644 --- a/src/lib/messaging/channels/openclaw-bridge-health.ts +++ b/src/lib/messaging/channels/openclaw-bridge-health.ts @@ -24,6 +24,7 @@ export type OpenClawBridgeHealthCommandRunner = ( export interface OpenClawBridgeHealthHookOptions { readonly sandboxName?: string; readonly executeSandboxCommand?: OpenClawBridgeHealthCommandRunner; + readonly beforeSuccess?: () => void; readonly log?: (message: string) => void; } @@ -120,6 +121,7 @@ export function createOpenClawBridgeHealthHook( } if (lines.some((line) => OPENCLAW_BRIDGE_POSITIVE_STARTUP_PATTERN.test(line))) { + options.beforeSuccess?.(); log(` ✓ '${spec.channelId}' bridge startup detected in sandbox runtime log.`); spec.onStartupDetected?.({ channelBlock, log }); return {}; diff --git a/src/lib/messaging/channels/wechat/hooks/ilink-login.ts b/src/lib/messaging/channels/wechat/hooks/ilink-login.ts index e648baf4366..1afd70f33da 100644 --- a/src/lib/messaging/channels/wechat/hooks/ilink-login.ts +++ b/src/lib/messaging/channels/wechat/hooks/ilink-login.ts @@ -32,6 +32,7 @@ export const WECHAT_ILINK_LOGIN_HOOK_ID = "wechat.ilinkLogin"; export interface WechatIlinkLoginHookOptions { readonly env?: NodeJS.ProcessEnv; readonly runLogin?: () => Promise; + readonly beforePersist?: () => void; readonly saveCredential?: (key: string, value: string) => void; readonly log?: (message: string) => void; } @@ -70,6 +71,7 @@ export function createWechatIlinkLoginHook( const { token, accountId, baseUrl, userId } = result.credentials; const normalizedBaseUrl = normalizeWechatIlinkBaseUrl(baseUrl); + options.beforePersist?.(); saveCredential("WECHAT_BOT_TOKEN", token); env.WECHAT_BOT_TOKEN = token; env.WECHAT_ACCOUNT_ID = accountId; diff --git a/test/channels/channels-add-bridge-lifecycle.test.ts b/test/channels/channels-add-bridge-lifecycle.test.ts index 534ec336765..84da75d0416 100644 --- a/test/channels/channels-add-bridge-lifecycle.test.ts +++ b/test/channels/channels-add-bridge-lifecycle.test.ts @@ -44,7 +44,6 @@ const GOOGLECHAT_ENV = { GOOGLECHAT_AUDIENCE: "https://bot.example.com/googlechat", GOOGLECHAT_APP_PRINCIPAL: "123456789012345678901", }; -const LIVE_IDENTITY_FINGERPRINT = "a".repeat(64); // Why this mock exists: the real googlechat tunnel/audience gate needs a human // operator (Google Cloud Console steps), so on a non-interactive test run it @@ -91,13 +90,8 @@ function printedText(): string { .join("\n"); } -function withoutGateway(args: readonly string[]): string[] { - const index = args[2] === "-g" ? 2 : args[3] === "-g" ? 3 : -1; - return index < 0 ? [...args] : [...args.slice(0, index), ...args.slice(index + 2)]; -} - function openshellCalls(): string[][] { - return runOpenshellSpy.mock.calls.map((call) => withoutGateway(call[0] as string[])); + return runOpenshellSpy.mock.calls.map((call) => call[0] as string[]); } beforeEach(() => { @@ -107,6 +101,9 @@ beforeEach(() => { process.env.HOME = testHome; process.env.NEMOCLAW_NON_INTERACTIVE = "1"; Object.assign(process.env, GOOGLECHAT_ENV); + vi.spyOn(policyChannelDependencies, "preflightSandboxPolicyAuthority").mockReturnValue( + "nemoclaw-managed", + ); logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); @@ -114,14 +111,7 @@ beforeEach(() => { throw new ExitError(code); }) as never); - registryEntry = { - name: "test-sb", - agent: "openclaw", - gatewayName: "nemoclaw", - lifecycleGeneration: "generation-1", - lifecycleLiveIdentityFingerprint: LIVE_IDENTITY_FINGERPRINT, - policies: [], - } as SandboxEntry; + registryEntry = { name: "test-sb", agent: "openclaw", policies: [] } as SandboxEntry; vi.spyOn(registry, "getSandbox").mockImplementation(() => registryEntry); vi.spyOn(registry, "listSandboxes").mockImplementation(() => ({ sandboxes: [registryEntry], @@ -139,6 +129,9 @@ beforeEach(() => { vi.spyOn(policies, "loadPresetForSandbox").mockReturnValue( "network_policies:\n stub:\n egress:\n - host: example.com\n", ); + vi.spyOn(policies, "getPresetContentGatewayState").mockImplementation(() => + appliedPresets.includes("googlechat") ? "match" : "absent", + ); vi.spyOn(policies, "applyPreset").mockImplementation((_sandboxName, preset) => { appliedPresets = [...new Set([...appliedPresets, preset])]; return true; @@ -167,40 +160,19 @@ beforeEach(() => { // crosses the direct channel action, generic provider upsert, and OpenShell // refresh boundary. Individual failure tests override the spy below. providerSpy = vi.spyOn(policyChannelDependencies, "upsertMessagingProviders"); - vi.spyOn( - policyChannelDependencies, - "revalidateChannelProviderPolicyAuthority", - ).mockImplementation(() => undefined); - vi.spyOn( - policyChannelDependencies, - "inspectMessagingProviderAttachmentTarget", - ).mockReturnValue(LIVE_IDENTITY_FINGERPRINT); vi.spyOn(policyChannelDependencies, "rebuildSandbox").mockImplementation(async () => undefined); stopGooglechatWebhookTunnelSpy = vi .spyOn(policyChannelDependencies, "stopGooglechatWebhookTunnel") .mockImplementation(() => undefined); - // Onboarding polls `provider refresh status` before creating the sandbox. - // Status-table columns: PROVIDER, CREDENTIAL_KEY, STRATEGY, STATUS. - const refreshStatusTable = (args: readonly string[]): string => - `${args[3] ?? ""} ${args[5] ?? ""} google-service-account-jwt refreshed\n`; - const isRefreshStatus = (args: readonly string[]): boolean => { - const command = withoutGateway(args); - return command[0] === "provider" && command[1] === "refresh" && command[2] === "status"; - }; - - runOpenshellSpy = vi.spyOn(runtime, "runOpenshell").mockImplementation((args) => { - const command = withoutGateway(args); - const providerMissing = command[0] === "provider" && command[1] === "get"; - return { - pid: 0, - output: [null, "", ""], - stdout: isRefreshStatus(args) ? refreshStatusTable(command) : "", - stderr: providerMissing ? `provider '${args[args.length - 1]}' not found` : "", - status: providerMissing ? 1 : 0, - signal: null, - }; - }); + runOpenshellSpy = vi.spyOn(runtime, "runOpenshell").mockImplementation((args) => ({ + pid: 0, + output: [null, "", ""], + stdout: "", + stderr: "", + status: args[0] === "provider" && args[1] === "get" ? 1 : 0, + signal: null, + })); const healthyGatewayState = { state: "healthy_named", @@ -247,13 +219,17 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { }, ], "nemoclaw", - { bestEffort: true, requireExactBindings: true }, + expect.objectContaining({ + bestEffort: true, + requireExactBindings: true, + revalidatePolicyRequirements: expect.any(Function), + }), ); const refreshCall = runOpenshellSpy.mock.calls.find( (call) => - withoutGateway(call[0] as string[]) - .slice(0, 3) - .join(" ") === "provider refresh configure", + (call[0] as string[])[0] === "provider" && + (call[0] as string[])[1] === "refresh" && + (call[0] as string[])[2] === "configure", ); expect(refreshCall).toBeDefined(); const refreshArgs = refreshCall?.[0] as string[]; diff --git a/test/channels/channels-add-preset.test.ts b/test/channels/channels-add-preset.test.ts index 5efdf77605e..44539293a4c 100644 --- a/test/channels/channels-add-preset.test.ts +++ b/test/channels/channels-add-preset.test.ts @@ -12,6 +12,7 @@ import { import { policyChannelDependencies } from "../../src/lib/actions/sandbox/policy-channel-dependencies"; import * as processRecovery from "../../src/lib/actions/sandbox/process-recovery"; import * as httpProbe from "../../src/lib/adapters/http/probe"; +import { PolicyAuthorityRefusalError } from "../../src/lib/adapters/openshell/policy-authority"; import * as runtime from "../../src/lib/adapters/openshell/runtime"; import * as store from "../../src/lib/credentials/store"; import * as gatewayRuntime from "../../src/lib/gateway-runtime-action"; @@ -187,6 +188,9 @@ beforeEach(() => { process.env.SLACK_BOT_TOKEN = "xoxb-slack-bot-token-for-test"; process.env.SLACK_APP_TOKEN = "xapp-slack-app-token-for-test"; process.env.DISCORD_BOT_TOKEN = "test-discord-token"; + vi.spyOn(policyChannelDependencies, "preflightSandboxPolicyAuthority").mockReturnValue( + "nemoclaw-managed", + ); sandboxAgent = "openclaw"; registryEntry = makeRegistryEntry(); @@ -213,10 +217,6 @@ beforeEach(() => { exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { throw new ExitError(code); }) as never); - vi.spyOn( - policyChannelDependencies, - "revalidateChannelProviderPolicyAuthority", - ).mockImplementation(() => undefined); vi.spyOn(registry, "getSandbox").mockImplementation(() => registryEntry); vi.spyOn(registry, "listSandboxes").mockImplementation(() => ({ @@ -347,6 +347,148 @@ afterEach(() => { }); describe("channels add applies a matching policy preset (#3437)", () => { + it("stops channel add before credential, provider, policy, or registry mutation when policy authority is inconclusive (#9833)", async () => { + vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority).mockImplementation(() => { + throw new Error("OpenShell returned an unknown policy source"); + }); + + await expectExit(() => addSandboxChannel("test-sb", { channel: "telegram" })); + + expect(printedText()).toContain("unknown policy source"); + expect(buildPlanSpy).not.toHaveBeenCalled(); + expect(getCredentialSpy).not.toHaveBeenCalled(); + expect(saveCredentialSpy).not.toHaveBeenCalled(); + expect(providerSpy).not.toHaveBeenCalled(); + expect(applyPresetSpy).not.toHaveBeenCalled(); + expect(updateSandboxSpy).not.toHaveBeenCalled(); + expect(rebuildSpy).not.toHaveBeenCalled(); + }); + + it("does not attribute an externally supplied channel preset to the onboard session (#9833)", async () => { + registryEntry = { ...registryEntry, policyAuthority: "externally-managed" }; + vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority).mockReturnValue( + "externally-managed", + ); + + await addSandboxChannel("test-sb", { channel: "telegram" }); + + expect(applyPresetSpy).toHaveBeenCalledWith("test-sb", "telegram", { + disclosedPresetState: "absent", + expectedExistingNetworkPolicyContent: null, + includeMessagingCredentialBindings: true, + }); + expect(sessionUpdates).toEqual([]); + expect(sessionState?.policyPresets).not.toContain("telegram"); + }); + + it("rolls back owned state when authority changes after provider registration (#9833)", async () => { + const refusal = new PolicyAuthorityRefusalError( + "OpenShell policy authority changed after provider registration", + ); + vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority).mockImplementation(() => + providerSpy.mock.calls.length > 0 + ? (() => { + throw refusal; + })() + : "nemoclaw-managed", + ); + + await expect(addSandboxChannel("test-sb", { channel: "telegram" })).rejects.toBe(refusal); + + expect(providerSpy).toHaveBeenCalledOnce(); + expect(applyPresetSpy).not.toHaveBeenCalled(); + expect(deleteCredentialSpy).toHaveBeenCalledWith("TELEGRAM_BOT_TOKEN"); + expect(runOpenshellSpy).toHaveBeenCalledWith( + ["sandbox", "provider", "detach", "test-sb", "test-sb-telegram-bridge"], + expect.anything(), + ); + expect(runOpenshellSpy).toHaveBeenCalledWith( + ["provider", "delete", "test-sb-telegram-bridge"], + expect.anything(), + ); + expect(updateSandboxSpy).toHaveBeenLastCalledWith("test-sb", { messaging: undefined }); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it("preserves live policy while rolling back owned state after authority drift (#9833)", async () => { + const refusal = new PolicyAuthorityRefusalError( + "OpenShell policy authority changed before channel plan persistence", + ); + vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority).mockImplementation(() => + applyPresetSpy.mock.calls.length > 0 + ? (() => { + throw refusal; + })() + : "nemoclaw-managed", + ); + + await expect(addSandboxChannel("test-sb", { channel: "telegram" })).rejects.toBe(refusal); + + expect(applyPresetSpy).toHaveBeenCalledOnce(); + expect(removePresetSpy).not.toHaveBeenCalled(); + expect(printedText()).toContain( + "Policy authority changed; the live 'telegram' policy was preserved", + ); + expect(deleteCredentialSpy).toHaveBeenCalledWith("TELEGRAM_BOT_TOKEN"); + expect(runOpenshellSpy).toHaveBeenCalledWith( + ["sandbox", "provider", "detach", "test-sb", "test-sb-telegram-bridge"], + expect.anything(), + ); + expect(runOpenshellSpy).toHaveBeenCalledWith( + ["provider", "delete", "test-sb-telegram-bridge"], + expect.anything(), + ); + expect(updateSandboxSpy).toHaveBeenLastCalledWith("test-sb", { messaging: undefined }); + expect(rebuildSpy).not.toHaveBeenCalled(); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it("refuses a drifted WhatsApp policy before channel attribution or gateway mutation (#9833)", async () => { + vi.mocked(policies.getPresetContentGatewayState).mockReturnValue("drift"); + + await expectExit(() => addSandboxChannel("test-sb", { channel: "whatsapp" })); + + expect(printedText()).toContain("No channel state was changed"); + expect(applyPresetSpy).not.toHaveBeenCalled(); + expect(providerSpy).not.toHaveBeenCalled(); + expect(updateSandboxSpy).not.toHaveBeenCalled(); + expect(rebuildSpy).not.toHaveBeenCalled(); + }); + + it("removes only newly adopted attribution for matching WhatsApp policy (#9833)", async () => { + registryEntry = { ...registryEntry, policies: ["npm"] }; + setSession("test-sb", ["pypi"]); + vi.mocked(policies.getPresetContentGatewayState).mockReturnValue("match"); + updateSandboxSpy.mockImplementation((_name, updates) => { + Object.assign(registryEntry, updates); + return true; + }); + const refusal = new PolicyAuthorityRefusalError( + "OpenShell policy authority changed after adopting the WhatsApp preset", + ); + const preflightAuthority = vi.mocked(policyChannelDependencies.preflightSandboxPolicyAuthority); + preflightAuthority.mockReturnValue("nemoclaw-managed"); + applyPresetSpy.mockImplementation((_name, presetName) => { + registryEntry.policies = [...(registryEntry.policies ?? []), presetName]; + registry.updateSandbox("test-sb", { policies: registryEntry.policies }); + preflightAuthority.mockImplementation(() => { + throw refusal; + }); + return true; + }); + + await expect(addSandboxChannel("test-sb", { channel: "whatsapp" })).rejects.toBe(refusal); + + expect(applyPresetSpy).toHaveBeenCalledWith("test-sb", "whatsapp", { + disclosedPresetState: "match", + expectedExistingNetworkPolicyContent: presetContent, + }); + expect(removePresetSpy).not.toHaveBeenCalled(); + expect(registryEntry.policies).toEqual(["npm"]); + expect(sessionState?.policyPresets).toEqual(["pypi"]); + expect(updateSandboxSpy).toHaveBeenCalledWith("test-sb", { policies: ["npm"] }); + }); + it("discloses token-channel egress before credential prompts and gateway mutation (#7179)", async () => { delete process.env.NEMOCLAW_NON_INTERACTIVE; delete process.env.TELEGRAM_BOT_TOKEN; @@ -421,7 +563,8 @@ describe("channels add applies a matching policy preset (#3437)", () => { expect(applyPresetSpy).toHaveBeenCalledOnce(); expect(applyPresetSpy).toHaveBeenCalledWith("test-sb", channel, { disclosedPresetState: "absent", - includeMessagingCredentialBindings: true, + expectedExistingNetworkPolicyContent: null, + includeMessagingCredentialBindings: true, }); expect(loadPresetForSandboxSpy).toHaveBeenCalledWith("test-sb", channel); expect(callOrder.indexOf(`applyPreset:${channel}`)).toBeLessThan( @@ -456,6 +599,7 @@ describe("channels add applies a matching policy preset (#3437)", () => { expect(applyPresetSpy).toHaveBeenCalledOnce(); expect(applyPresetSpy).toHaveBeenCalledWith("test-sb", "whatsapp", { disclosedPresetState: "absent", + expectedExistingNetworkPolicyContent: null, includeMessagingCredentialBindings: true, }); expect(callOrder.indexOf("scopeDisclosure")).toBeLessThan(callOrder.indexOf("updateSandbox")); @@ -472,9 +616,10 @@ describe("channels add applies a matching policy preset (#3437)", () => { await expectExit(() => addSandboxChannel("test-sb", { channel: "whatsapp" })); expect(providerSpy).not.toHaveBeenCalled(); - expect(updateSandboxSpy).not.toHaveBeenCalled(); + expect(updateSandboxSpy).toHaveBeenLastCalledWith("test-sb", { messaging: undefined }); expect(applyPresetSpy).toHaveBeenCalledWith("test-sb", "whatsapp", { disclosedPresetState: "absent", + expectedExistingNetworkPolicyContent: null, includeMessagingCredentialBindings: true, }); expect(callOrder).not.toContain("promptAndRebuild"); @@ -574,15 +719,16 @@ describe("channels add applies a matching policy preset (#3437)", () => { expect(applyPresetSpy).toHaveBeenCalledWith("test-sb", "telegram", { disclosedPresetState: "absent", + expectedExistingNetworkPolicyContent: null, includeMessagingCredentialBindings: true, }); - expect(updateSandboxSpy).not.toHaveBeenCalled(); + expect(updateSandboxSpy).toHaveBeenLastCalledWith("test-sb", { messaging: undefined }); expect(deleteCredentialSpy).toHaveBeenCalledWith("TELEGRAM_BOT_TOKEN"); expect(sessionUpdates).toEqual([]); expect(callOrder).not.toContain("promptAndRebuild"); }); - it("keeps plan state and skips provider delete when rollback detach fails", async () => { + it("leaves plan state untouched and reports residual gateway state when detach fails", async () => { applyPresetResult = false; runOpenshellSpy.mockImplementation((args: string[]) => args.slice(0, 3).join(" ") === "sandbox provider detach" @@ -592,15 +738,10 @@ describe("channels add applies a matching policy preset (#3437)", () => { await expectExit(() => addSandboxChannel("test-sb", { channel: "telegram" })); - expect(updateSandboxSpy).not.toHaveBeenCalled(); + expect(updateSandboxSpy).toHaveBeenLastCalledWith("test-sb", { messaging: undefined }); expect(deleteCredentialSpy).toHaveBeenCalledWith("TELEGRAM_BOT_TOKEN"); - expect(runOpenshellSpy.mock.calls.map(([args]) => args)).not.toContainEqual([ - "provider", - "delete", - "test-sb-telegram-bridge", - ]); expect(printedText()).toContain("Rollback could not fully clean gateway-providers"); - expect(printedText()).toContain("'nemoclaw test-sb channels remove telegram'"); + expect(printedText()).toContain("inspect the live policy and local sandbox state"); expect(callOrder).not.toContain("promptAndRebuild"); }); @@ -613,7 +754,9 @@ describe("channels add applies a matching policy preset (#3437)", () => { await expectExit(() => addSandboxChannel("test-sb", { channel: "telegram" })); - expect(updateSandboxSpy).not.toHaveBeenCalled(); + expect(updateSandboxSpy).toHaveBeenLastCalledWith("test-sb", { + messaging: registryEntry.messaging, + }); expect(saveCredentialSpy).toHaveBeenCalledWith("TELEGRAM_BOT_TOKEN", "prior-telegram-token"); expect(providerSpy).toHaveBeenCalledTimes(2); expect( @@ -625,6 +768,80 @@ describe("channels add applies a matching policy preset (#3437)", () => { expect(printedText()).toContain("Rollback could not fully clean gateway-providers"); }); + it("registers Hermes Discord with the exact static provider binding", async () => { + sandboxAgent = "hermes"; + registryEntry = makeRegistryEntry([], [], "hermes"); + + await addSandboxChannel("test-sb", { channel: "discord" }); + + expect(providerSpy).toHaveBeenCalledWith( + [ + { + name: "test-sb-discord-bridge", + envKey: "DISCORD_BOT_TOKEN", + token: "test-discord-token", + providerType: "discord-hermes-static-v1", + }, + ], + expect.objectContaining({ + bestEffort: true, + requireExactBindings: true, + revalidatePolicyRequirements: expect.any(Function), + }), + ); + }); + + it("does not persist credentials after a provider identity conflict", async () => { + providerSpy.mockImplementationOnce(() => { + throw Object.assign(new Error("provider binding conflict"), { + code: "NEMOCLAW_MESSAGING_PROVIDER_BINDING_CONFLICT", + mutatedProviderNames: [], + }); + }); + + await expectExit(() => addSandboxChannel("test-sb", { channel: "slack" })); + + expect(providerSpy.mock.calls[0]?.[0]).toHaveLength(2); + expect(saveCredentialSpy).not.toHaveBeenCalled(); + expect(applyPresetSpy).not.toHaveBeenCalled(); + expect(updateSandboxSpy).not.toHaveBeenCalled(); + expect(rebuildSpy).not.toHaveBeenCalled(); + expect( + runOpenshellSpy.mock.calls.filter(([args]) => + (args as string[]).some((arg) => arg === "detach" || arg === "delete"), + ), + ).toEqual([]); + }); + + it("restores Hermes Discord credentials through the exact static provider binding (#9833)", async () => { + applyPresetResult = false; + sandboxAgent = "hermes"; + registryEntry = makeRegistryEntry(["discord"], [], "hermes"); + getCredentialSpy.mockImplementation((key: string) => + key === "DISCORD_BOT_TOKEN" ? "prior-discord-token" : null, + ); + + await expectExit(() => addSandboxChannel("test-sb", { channel: "discord" })); + + expect(providerSpy).toHaveBeenCalledTimes(2); + expect(providerSpy).toHaveBeenNthCalledWith( + 2, + [ + { + name: "test-sb-discord-bridge", + envKey: "DISCORD_BOT_TOKEN", + token: "prior-discord-token", + providerType: "discord-hermes-static-v1", + }, + ], + { bestEffort: true, requireExactBindings: true }, + ); + expect(saveCredentialSpy.mock.calls).toEqual([ + ["DISCORD_BOT_TOKEN", "test-discord-token"], + ["DISCORD_BOT_TOKEN", "prior-discord-token"], + ]); + }); + it("leaves prior plan state untouched even when re-upsert during re-add rollback throws", async () => { applyPresetResult = false; registryEntry = makeRegistryEntry(["telegram"]); @@ -639,7 +856,9 @@ describe("channels add applies a matching policy preset (#3437)", () => { await expectExit(() => addSandboxChannel("test-sb", { channel: "telegram" })); - expect(updateSandboxSpy).not.toHaveBeenCalled(); + expect(updateSandboxSpy).toHaveBeenLastCalledWith("test-sb", { + messaging: registryEntry.messaging, + }); expect(saveCredentialSpy).toHaveBeenCalledWith("TELEGRAM_BOT_TOKEN", "prior-telegram-token"); expect(printedText()).toContain("Failed to restore gateway providers for 'telegram'"); expect(printedText()).toContain("Rollback could not fully clean gateway-providers"); @@ -735,6 +954,7 @@ describe("channels add/remove keeps session.policyPresets in sync with registry" expect(applyPresetSpy).toHaveBeenCalledWith("test-sb", "slack", { disclosedPresetState: "absent", + expectedExistingNetworkPolicyContent: null, includeMessagingCredentialBindings: true, }); expect(sessionUpdates).toEqual([]); @@ -748,6 +968,7 @@ describe("channels add/remove keeps session.policyPresets in sync with registry" expect(applyPresetSpy).toHaveBeenCalledWith("test-sb", "slack", { disclosedPresetState: "absent", + expectedExistingNetworkPolicyContent: null, includeMessagingCredentialBindings: true, }); expect(sessionUpdates).toEqual([]); @@ -761,6 +982,7 @@ describe("channels add/remove keeps session.policyPresets in sync with registry" expect(applyPresetSpy).toHaveBeenCalledWith("test-sb", "slack", { disclosedPresetState: "absent", + expectedExistingNetworkPolicyContent: null, includeMessagingCredentialBindings: true, }); expect(callOrder).toContain("promptAndRebuild"); @@ -890,7 +1112,7 @@ describe("channel preset source-of-truth", () => { it.each(knownChannelNames())( "channel $name ships a preset that parsePresetPolicyKeys accepts", (name) => { - const content = policies.loadPresetForSandbox("test-sb", name); + const content = policies.loadPreset(name); expect(content, `${name}: preset YAML not found on disk`).not.toBeNull(); expect( policies.parsePresetPolicyKeys(content!).length, diff --git a/test/channels/channels-remove-full-teardown.test.ts b/test/channels/channels-remove-full-teardown.test.ts index 14a61458c42..c060fd16ce1 100644 --- a/test/channels/channels-remove-full-teardown.test.ts +++ b/test/channels/channels-remove-full-teardown.test.ts @@ -185,6 +185,9 @@ process.exit = (code) => { throw new Error("__PROCESS_EXIT__:" + code); }; +const policyAuthorityPreflight = require(${j("actions/sandbox/policy-authority/preflight.js")}); +policyAuthorityPreflight.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; + const channelModule = require(${j("actions/sandbox/policy-channel.js")}); module.exports = {