diff --git a/docs/manage-sandboxes/enable-channels-during-onboarding.mdx b/docs/manage-sandboxes/enable-channels-during-onboarding.mdx index 05db89ad09b..1c9a9ebfd7c 100644 --- a/docs/manage-sandboxes/enable-channels-during-onboarding.mdx +++ b/docs/manage-sandboxes/enable-channels-during-onboarding.mdx @@ -4,7 +4,7 @@ title: "Enable Channels During Onboarding" sidebar-title: "Enable Channels During Onboarding" description: "Select messaging channels and supply their credentials or pairing inputs during NemoClaw onboarding." -description-agent: "Explains the interactive and scripted onboarding flows for selecting messaging channels, creating OpenShell bridge providers, and removing a channel with the channel lifecycle commands. Use when enabling or disabling channels during onboarding." +description-agent: "Explains the interactive and scripted onboarding flows for selecting messaging channels, creating OpenShell bridge providers, preserving reusable channels, and using lifecycle commands to stop or explicitly remove a channel. Use when enabling or disabling channels during onboarding." keywords: ["nemoclaw onboard messaging", "messaging channel picker", "channel environment variables", "disable messaging channel"] content: type: "how_to" @@ -89,9 +89,8 @@ Credential bindings remain OpenShell credential placeholders, so raw messaging c ## Remove a Channel -Use `channels remove` when you want to delete a channel's OpenShell provider, runtime configuration, and matching network policy. -This action can also clear stored pairing state. -Use `channels stop` when you want to pause the channel without deleting credentials or pairing state. +Use `channels stop` when you want to pause a channel without deleting its credentials or pairing state. +Use `channels remove` for an explicit, durable removal of its selection, OpenShell provider, runtime configuration, and matching network policy preset: ```bash $$nemoclaw channels remove @@ -100,11 +99,13 @@ $$nemoclaw channels remove Accept the rebuild to remove the channel configuration and its network policy preset from the replacement sandbox. When onboarding runs without a terminal on stdin or with `NEMOCLAW_NON_INTERACTIVE=1`, NemoClaw queues the removal. Run `$$nemoclaw rebuild` to apply it. -Clearing the channel's host environment variables is not a removal signal when the matching provider remains in OpenShell. -Onboarding reuses that provider and keeps the channel selected, including its network egress. -For a QR-paired channel such as WhatsApp, only `channels remove` clears the in-sandbox pairing state. -Refer to [Manage Messaging Channels](manage-messaging-channels) for channel-specific removal effects and recovery steps. +Clearing a channel's host environment variables is not a removal signal when its recorded OpenShell gateway provider still matches the channel's credential contract. +Onboarding preserves the channel selection and matching network policy preset because interactive inputs normally disappear between runs. +If a required gateway provider is missing or no longer matches the expected credential contract, onboarding disables the channel and drops the matching policy preset. + +For an in-sandbox QR-paired channel such as WhatsApp, only `channels remove` clears its session or pairing state before teardown. +Refer to [Manage Messaging Channels](manage-messaging-channels) for channel-specific removal effects and recovery guidance. ## Verify the Result diff --git a/src/lib/adapters/openshell/provider-profile.ts b/src/lib/adapters/openshell/provider-profile.ts index ec9ae7e71a5..c96e961ab51 100644 --- a/src/lib/adapters/openshell/provider-profile.ts +++ b/src/lib/adapters/openshell/provider-profile.ts @@ -86,18 +86,26 @@ export function parseCheckedInProviderProfileContract( } /** Compare an exported gateway profile with its checked-in credential boundary. */ -export function exportedProviderProfileMatchesContract( +export function compareExportedProviderProfileWithContract( exported: string, expected: CheckedInProviderProfileContract, -): boolean { +): boolean | null { try { const actual = providerProfileBoundary(JSON.parse(exported) as unknown); - return actual !== null && isDeepStrictEqual(actual, expected.boundary); + return actual === null ? null : isDeepStrictEqual(actual, expected.boundary); } catch { - return false; + return null; } } +/** Compare an exported gateway profile with its checked-in credential boundary. */ +export function exportedProviderProfileMatchesContract( + exported: string, + expected: CheckedInProviderProfileContract, +): boolean { + return compareExportedProviderProfileWithContract(exported, expected) === true; +} + export function isMissingProviderProfile(output: string, profileId: string): boolean { const normalized = output .replace(/\u001b\[[0-?]*[ -/]*[@-~]/gu, "") diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 44fb3caf150..694b2a6ebeb 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -3270,8 +3270,8 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { loadSession: onboardSession.loadSession, getActiveSandbox: (name) => registry.getSandbox(name), mergePolicyMessagingChannels, - detectUnconfiguredMessagingChannels: - messagingChannelSetup.detectUnconfiguredMessagingChannels, + detectUnconfiguredMessagingChannels: messagingChannelSetup.detectUnconfiguredMessagingChannels, + providerMatchesGatewayCredential, verifyCompatibleEndpointSandboxSmoke: (options) => verifyCompatibleEndpointSandboxSmoke({ ...options, diff --git a/src/lib/onboard/credential-provider-registration.test.ts b/src/lib/onboard/credential-provider-registration.test.ts index 3cf7a4cc51d..02c59b3a63f 100644 --- a/src/lib/onboard/credential-provider-registration.test.ts +++ b/src/lib/onboard/credential-provider-registration.test.ts @@ -71,10 +71,8 @@ function providerMetadata( return { status: 0, stdout: [ - `Id: provider-${name}`, `Name: ${name}`, `Type: ${type}`, - "Resource version: 1", `Credential keys: ${credentialKey}`, "Config keys: ", ].join("\n"), @@ -358,6 +356,53 @@ describe("credential provider registration", () => { ).toEqual({ kind: "indeterminate" }); }); + it("does not classify an unavailable provider inspection as a missing binding", () => { + const session = { stagedCredentialProviders: [] } as unknown as Session; + const runOpenshell = vi.fn((_args: string[]) => ({ + status: 1, + stdout: "", + stderr: "gateway unavailable", + })); + const registration = createCredentialProviderRegistration( + registrationDeps(runOpenshell, session), + ); + + expect(() => + registration.providerMatchesGatewayCredential( + "alpha-discord-bridge", + "generic", + "DISCORD_BOT_TOKEN", + ), + ).toThrow("Could not inspect credential provider 'alpha-discord-bridge'"); + expect(runOpenshell).toHaveBeenCalledWith( + ["provider", "get", "-g", "test-gateway", "alpha-discord-bridge"], + expect.objectContaining({ ignoreError: true, suppressOutput: true }), + ); + }); + + it("does not classify an unavailable static profile inspection as profile drift", () => { + const session = { stagedCredentialProviders: [] } as unknown as Session; + const runOpenshell = vi.fn((_args: string[]) => ({ + status: 1, + stdout: "", + stderr: "gateway unavailable", + })); + const deps = registrationDeps(runOpenshell, session); + deps.root = process.cwd(); + const registration = createCredentialProviderRegistration(deps); + + expect(() => + registration.providerMatchesGatewayCredential( + "alpha-discord-bridge", + "discord-hermes-static-v1", + "DISCORD_BOT_TOKEN", + ), + ).toThrow("Could not inspect static provider profile 'discord-hermes-static-v1'"); + expect(runOpenshell.mock.calls.map(([args]) => args.join(" "))).toEqual([ + "provider profile -g test-gateway export discord-hermes-static-v1 --output json", + ]); + }); + it("rejects tokenless Hermes Discord profile drift before provider mutation", async () => { const session = { stagedCredentialProviders: [] } as unknown as Session; const runOpenshell = vi.fn((args: string[]) => diff --git a/src/lib/onboard/credential-provider-registration.ts b/src/lib/onboard/credential-provider-registration.ts index cbb137ba3fd..196ed27c247 100644 --- a/src/lib/onboard/credential-provider-registration.ts +++ b/src/lib/onboard/credential-provider-registration.ts @@ -12,6 +12,17 @@ import { createGatewayScopedOpenshellRunner } from "./setup-inference"; const providers = require("./providers"); +/** Late-bound provider upsert seam used by live credential fixtures. */ +export const credentialProviderRegistrationDependencies = { + upsertMessagingProviders( + tokenDefs: MessagingTokenDef[], + runOpenshell: OpenshellCliHelpers["runOpenshell"], + options: MessagingProviderRegistrationOptions, + ): string[] { + return providers.upsertMessagingProviders(tokenDefs, runOpenshell, options) as string[]; + }, +}; + export interface StageSandboxCredentialProvidersInput { sandboxName: string; enabledChannels: readonly string[]; @@ -175,17 +186,16 @@ export function createCredentialProviderRegistration(deps: CredentialProviderReg return result; } - function upsertMessagingProvidersAtGateway( + function upsertMessagingProviders( tokenDefs: MessagingTokenDef[], - options: MessagingProviderRegistrationOptions, - gatewayName: string, - runOpenshell: OpenshellCliHelpers["runOpenshell"] = gatewayRunner(gatewayName), + options: MessagingProviderRegistrationOptions = {}, + runOpenshell: OpenshellCliHelpers["runOpenshell"] = deps.runOpenshell, ): string[] { - const upserted = providers.upsertMessagingProviders( + const upserted = credentialProviderRegistrationDependencies.upsertMessagingProviders( tokenDefs, runOpenshell, - { ...options, gatewayName }, - ) as string[]; + options, + ); recordMigratedLegacyMessagingCredentials( tokenDefs, upserted, @@ -195,31 +205,22 @@ export function createCredentialProviderRegistration(deps: CredentialProviderReg return upserted; } - function upsertMessagingProviders( - tokenDefs: MessagingTokenDef[], - options: MessagingProviderRegistrationOptions = {}, - ): string[] { - const gatewayName = deps.getGatewayName(); - return upsertMessagingProvidersAtGateway(tokenDefs, options, gatewayName); - } - function credentialBindingMatchesGateway( binding: CheckpointProviderBinding, runOpenshell: OpenshellCliHelpers["runOpenshell"], ): boolean { - return inspectGatewayCredentialBinding(binding, runOpenshell).kind === "exact"; - } - - function inspectGatewayCredentialBinding( - binding: CheckpointProviderBinding, - runOpenshell: OpenshellCliHelpers["runOpenshell"], - ): gatewayProviderMetadata.GatewayCredentialOnlyProviderInspection { - const profileMatches = messagingBridgeProvider.matchesRegisteredMessagingBridgeProfile( + const staticProfile = messagingBridgeProvider.inspectRegisteredStaticMessagingProfile( binding.type, { root: deps.root, runOpenshell }, ); - if (profileMatches === false) return { kind: "indeterminate" }; - return gatewayProviderMetadata.inspectGatewayCredentialFamilyProviderBinding( + if (staticProfile.kind === "indeterminate") { + throw new Error( + `Could not inspect static provider profile '${binding.type}' on OpenShell gateway '${deps.getGatewayName()}'. Verify the gateway is reachable, then retry the command.`, + ); + } + if (staticProfile.kind === "collision") return false; + + const provider = gatewayProviderMetadata.inspectGatewayCredentialFamilyProviderBinding( { name: binding.name, type: binding.type, @@ -227,14 +228,12 @@ export function createCredentialProviderRegistration(deps: CredentialProviderReg }, runOpenshell, ); - } - - function inspectGatewayCredential( - name: string, - type: string, - credentialEnv: string, - ): gatewayProviderMetadata.GatewayCredentialOnlyProviderInspection { - return inspectGatewayCredentialBinding({ name, type, credentialEnv }, gatewayRunner()); + if (provider.kind === "indeterminate") { + throw new Error( + `Could not inspect credential provider '${binding.name}' on OpenShell gateway '${deps.getGatewayName()}'. Verify the gateway is reachable, then retry the command.`, + ); + } + return provider.kind === "exact"; } function providerMatchesGatewayCredential( @@ -283,8 +282,7 @@ export function createCredentialProviderRegistration(deps: CredentialProviderReg messaging.messagingTokenDefs.map((tokenDef) => [tokenDef.name, tokenDef]), ); const tokenDefs = messaging.messagingTokenDefs.filter(hasConfiguredMessagingCredential); - const gatewayName = deps.getGatewayName(); - const runOpenshell = gatewayRunner(gatewayName); + const runOpenshell = gatewayRunner(); preflightRequiredCredentialProviderBindings( input.requiredBindings, plannedTokenDefs, @@ -297,14 +295,13 @@ export function createCredentialProviderRegistration(deps: CredentialProviderReg false, deps, ); - const registered = upsertMessagingProvidersAtGateway( + const registered = upsertMessagingProviders( tokenDefs, { replaceExisting: input.replaceExisting === true, allowedSandboxes: input.replaceExisting === true ? [input.sandboxName] : undefined, revalidateSandboxIdentity: input.revalidateSandboxIdentity, }, - gatewayName, runOpenshell, ); input.revalidateSandboxIdentity?.("record staged credential provider receipts"); @@ -317,7 +314,6 @@ export function createCredentialProviderRegistration(deps: CredentialProviderReg } return { - inspectGatewayCredential, providerMatchesGatewayCredential, stageSandboxCredentialProviders, upsertProvider, diff --git a/src/lib/onboard/machine/handlers/policies-test-fixture.ts b/src/lib/onboard/machine/handlers/policies-test-fixture.ts index a39cdaf7835..cb7d649afdc 100644 --- a/src/lib/onboard/machine/handlers/policies-test-fixture.ts +++ b/src/lib/onboard/machine/handlers/policies-test-fixture.ts @@ -4,6 +4,7 @@ import { vi } from "vitest"; import { makeMessagingPlan } from "../../../../../test/helpers/messaging-plan-fixtures"; +import { mergePolicyMessagingChannels } from "../../messaging-policy-presets"; import { createSession, type Session, type SessionUpdates } from "../../../state/onboard-session"; import type { PoliciesStateOptions } from "./policies"; @@ -19,13 +20,11 @@ export function createPolicyHandlerDeps( activeSandbox: vi.fn(() => ({ messaging: { plan: makeMessagingPlan({ channels: ["telegram"] }) }, })), - mergeChannels: vi.fn( - (selected: string[], recorded: string[], active: string[] | null | undefined) => - selected.length > 0 ? selected : (active ?? recorded), - ), + mergeChannels: vi.fn(mergePolicyMessagingChannels), unconfiguredChannels: vi.fn( (_planChannels: readonly string[], _selectedChannels: readonly string[]) => [] as string[], ), + providerMatchesGatewayCredential: vi.fn(() => false), smoke: vi.fn(), prepareResume: vi.fn( ( @@ -61,6 +60,7 @@ export function createPolicyHandlerDeps( getActiveSandbox: calls.activeSandbox, mergePolicyMessagingChannels: calls.mergeChannels, detectUnconfiguredMessagingChannels: calls.unconfiguredChannels, + providerMatchesGatewayCredential: calls.providerMatchesGatewayCredential, verifyCompatibleEndpointSandboxSmoke: calls.smoke, preparePolicyPresetResumeSelection: calls.prepareResume, arePolicyPresetsApplied: calls.appliedCheck, diff --git a/src/lib/onboard/machine/handlers/policies.test.ts b/src/lib/onboard/machine/handlers/policies.test.ts index 80f798f5372..d3129577bda 100644 --- a/src/lib/onboard/machine/handlers/policies.test.ts +++ b/src/lib/onboard/machine/handlers/policies.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it, vi } from "vitest"; +import { makeMessagingPlan } from "../../../../../test/helpers/messaging-plan-fixtures"; import { createPolicyHandlerDeps, basePolicyHandlerOptions } from "./policies-test-fixture"; import { handlePoliciesState } from "./policies"; @@ -76,6 +77,164 @@ describe("policy state handler", () => { expect(result.appliedPolicyPresets).toEqual([]); }); + it("keeps a channel in policy requirements when every credential binding matches its gateway provider (#10667)", async () => { + const discordPlan = makeMessagingPlan({ + channels: ["discord"], + agent: "hermes", + credentialBindings: [ + { + channelId: "discord", + credentialId: "discordBotToken", + sourceInput: "botToken", + providerName: "my-assistant-discord-bridge", + providerEnvKey: "DISCORD_BOT_TOKEN", + placeholder: "openshell:resolve:env:DISCORD_BOT_TOKEN", + credentialAvailable: true, + credentialHash: "discord-token-hash", + }, + ], + }); + const providerMatcher = vi.fn( + (name: string, type: string, credentialEnv: string) => + name === "my-assistant-discord-bridge" && + type === "discord-hermes-static-v1" && + credentialEnv === "DISCORD_BOT_TOKEN", + ); + const { deps, calls } = createPolicyHandlerDeps({ + getActiveSandbox: vi.fn(() => ({ messaging: { plan: discordPlan } })), + providerMatchesGatewayCredential: providerMatcher, + }); + calls.unconfiguredChannels.mockImplementation((_planChannels, configuredChannels) => + configuredChannels.includes("discord") ? [] : ["discord"], + ); + + await handlePoliciesState({ + ...basePolicyHandlerOptions(deps), + selectedMessagingChannels: [], + agent: { name: "hermes" }, + }); + + expect(calls.unconfiguredChannels).toHaveBeenCalledWith(["discord"], ["discord"], { + name: "hermes", + }); + expect(providerMatcher).toHaveBeenCalledExactlyOnceWith( + "my-assistant-discord-bridge", + "discord-hermes-static-v1", + "DISCORD_BOT_TOKEN", + ); + expect(calls.mergeChannels).toHaveBeenCalledWith([], [], ["discord"], []); + expect(calls.setupPolicies).toHaveBeenCalledWith( + "my-assistant", + expect.objectContaining({ enabledChannels: ["discord"], disabledChannels: [] }), + ); + }); + + it.each([ + { + condition: "both bindings match", + providerMatchesGatewayCredential: () => true, + expectedEnabled: ["slack"], + expectedDisabled: [] as string[], + }, + { + condition: "one binding is missing", + providerMatchesGatewayCredential: (_name: string, _type: string, credentialEnv: string) => + credentialEnv === "SLACK_BOT_TOKEN", + expectedEnabled: [] as string[], + expectedDisabled: ["slack"], + }, + ])( + "requires every Slack binding before retaining its policy when $condition (#10667)", + async ({ providerMatchesGatewayCredential, expectedEnabled, expectedDisabled }) => { + const slackPlan = makeMessagingPlan({ + channels: ["slack"], + agent: "hermes", + credentialBindings: [ + { + channelId: "slack", + credentialId: "slackBotToken", + sourceInput: "botToken", + providerName: "my-assistant-slack-bridge", + providerEnvKey: "SLACK_BOT_TOKEN", + placeholder: "openshell:resolve:env:SLACK_BOT_TOKEN", + credentialAvailable: true, + }, + { + channelId: "slack", + credentialId: "slackAppToken", + sourceInput: "appToken", + providerName: "my-assistant-slack-app", + providerEnvKey: "SLACK_APP_TOKEN", + placeholder: "openshell:resolve:env:SLACK_APP_TOKEN", + credentialAvailable: true, + }, + ], + }); + const providerMatcher = vi.fn(providerMatchesGatewayCredential); + const { deps, calls } = createPolicyHandlerDeps({ + getActiveSandbox: vi.fn(() => ({ messaging: { plan: slackPlan } })), + providerMatchesGatewayCredential: providerMatcher, + }); + calls.unconfiguredChannels.mockImplementation((_planChannels, configuredChannels) => + configuredChannels.includes("slack") ? [] : ["slack"], + ); + + await handlePoliciesState({ + ...basePolicyHandlerOptions(deps), + selectedMessagingChannels: [], + agent: { name: "hermes" }, + }); + + expect(providerMatcher).toHaveBeenCalledTimes(2); + expect(calls.setupPolicies).toHaveBeenCalledWith( + "my-assistant", + expect.objectContaining({ + enabledChannels: expectedEnabled, + disabledChannels: expectedDisabled, + }), + ); + }, + ); + + it.each([ + ["openclaw", "google-chat-bridge"], + ["hermes", "google-chat-hermes-bridge"], + ] as const)( + "keeps Google Chat in %s policy requirements when its gateway-minted bridge provider matches", + async (agent, providerType) => { + const googlechatPlan = makeMessagingPlan({ channels: ["googlechat"], agent }); + const providerMatcher = vi.fn( + (name: string, type: string, credentialEnv: string) => + name === "my-assistant-googlechat-bridge" && + type === providerType && + credentialEnv === "GOOGLE_CHAT_ACCESS_TOKEN", + ); + const { deps, calls } = createPolicyHandlerDeps({ + getActiveSandbox: vi.fn(() => ({ messaging: { plan: googlechatPlan } })), + providerMatchesGatewayCredential: providerMatcher, + }); + calls.unconfiguredChannels.mockImplementation((_planChannels, configuredChannels) => + configuredChannels.includes("googlechat") ? [] : ["googlechat"], + ); + + await handlePoliciesState({ + ...basePolicyHandlerOptions(deps), + selectedMessagingChannels: [], + agent: { name: agent }, + }); + + expect(providerMatcher).toHaveBeenCalledExactlyOnceWith( + "my-assistant-googlechat-bridge", + providerType, + "GOOGLE_CHAT_ACCESS_TOKEN", + ); + expect(calls.setupPolicies).toHaveBeenCalledWith( + "my-assistant", + expect.objectContaining({ enabledChannels: ["googlechat"], disabledChannels: [] }), + ); + }, + ); + it("merges live messaging channels into policy requirements", async () => { const { deps, calls } = createPolicyHandlerDeps(); await handlePoliciesState({ diff --git a/src/lib/onboard/machine/handlers/policies.ts b/src/lib/onboard/machine/handlers/policies.ts index 5939937cea5..5283c4e7ae4 100644 --- a/src/lib/onboard/machine/handlers/policies.ts +++ b/src/lib/onboard/machine/handlers/policies.ts @@ -7,6 +7,7 @@ import { normalizeAgentNameForResumeState } from "../../agent-resume-state"; import { getActiveChannelsFromPlan, getDisabledChannelsFromPlan, + messagingChannelsWithReusableGatewayCredentials, } from "../../messaging-plan-session"; import type { HostLocalInferenceSandboxProofAuthority } from "../../runtime-provider/host-local-inference-routing"; import { advanceTo, type OnboardStateTransitionResult } from "../result"; @@ -57,6 +58,7 @@ export interface PoliciesStateOptions { selectedChannels: readonly string[], agent: Agent, ): string[]; + providerMatchesGatewayCredential(name: string, type: string, credentialEnv: string): boolean; verifyCompatibleEndpointSandboxSmoke(options: { sandboxName: string; provider: string; @@ -148,15 +150,19 @@ export async function handlePoliciesState({ const activePlan = activeSandbox?.messaging?.plan; const activeMessagingChannels = getActiveChannelsFromPlan(activePlan); const planDisabledChannels = getDisabledChannelsFromPlan(activePlan); - // A channel the operator stopped configuring never reaches `disabledChannels`, - // so without this the reused plan keeps it enabled and every later onboarding - // run re-applies its egress preset. Adding it to `disabledChannels` here lets - // the existing disabled-channel pruning drop the preset from both the merged - // selection and the previously-applied set. - // + const reusableMessagingChannels = messagingChannelsWithReusableGatewayCredentials( + activePlan ?? latestSession?.messagingPlan ?? null, + deps.providerMatchesGatewayCredential, + ); + // An active host-backed channel remains selected only while every recorded + // credential binding, or its gateway-minted bridge provider, still matches. + // Missing process inputs alone do not disable it because interactive values + // normally disappear between onboard runs. A missing or mismatched provider + // adds the channel to `disabledChannels`, so existing pruning removes its + // preset from the merged and previously applied sets. const unconfiguredMessagingChannels = deps.detectUnconfiguredMessagingChannels( [...recordedMessagingChannels, ...activeMessagingChannels], - selectedMessagingChannels, + [...new Set([...selectedMessagingChannels, ...reusableMessagingChannels])], agent, ); const disabledChannels = diff --git a/src/lib/onboard/machine/handlers/sandbox.ts b/src/lib/onboard/machine/handlers/sandbox.ts index df3f92931e1..4c96ab7cfb6 100644 --- a/src/lib/onboard/machine/handlers/sandbox.ts +++ b/src/lib/onboard/machine/handlers/sandbox.ts @@ -1092,7 +1092,9 @@ class SandboxStateFlow< return { ...state, session }; } - private assertGatewayRouteCompatible(sandboxName: string | null): void { + private assertGatewayRouteCompatible( + sandboxName: string | null, + ): asserts sandboxName is string { const targetEntry = sandboxName ? this.deps.getSandboxRegistryEntry(sandboxName) : null; if (!sandboxName || !targetEntry) { this.failGatewayRouteCheck( diff --git a/src/lib/onboard/messaging-bridge-provider.test.ts b/src/lib/onboard/messaging-bridge-provider.test.ts index 234f932ecae..537fd5245f9 100644 --- a/src/lib/onboard/messaging-bridge-provider.test.ts +++ b/src/lib/onboard/messaging-bridge-provider.test.ts @@ -4,15 +4,14 @@ import { describe, expect, it, vi } from "vitest"; import YAML from "yaml"; import type { ChannelManifest } from "../messaging/manifest"; -import { redactFull } from "../security/redact"; import { bridgeProviderNamesForChannel, bridgeSecretEnvsForChannel, collectMessagingBridgeTokenDefs, configureMessagingBridgeRefreshes, ensureMessagingBridgeProfiles, + inspectRegisteredStaticMessagingProfile, listMessagingBridgeProfiles, - matchesRegisteredMessagingBridgeProfile, MESSAGING_BRIDGE_PENDING_VALUE, type MessagingBridgeProfile, refreshStatusForCredential, @@ -23,6 +22,7 @@ const SA_JSON = JSON.stringify({ private_key: "fake-test-private-key-material", }); const normalizeCredentialValue = (v: unknown) => String(v ?? "").trim(); +const redact = (s: string) => s; const noLog = vi.fn(); // `openshell provider refresh status` output as the CLI prints it, so the parser @@ -50,32 +50,6 @@ const GC_PROFILE: MessagingBridgeProfile = { sourceSecretEnv: "GOOGLECHAT_SERVICE_ACCOUNT", }; -const GC_PROFILE_DOC = { - id: GC_PROFILE.profileId, - credentials: [ - { - name: "access_token", - env_vars: [GC_PROFILE.credentialKey], - required: true, - auth_style: "bearer", - header_name: "Authorization", - query_param: "", - refresh: { - strategy: GC_PROFILE.strategy, - scopes: GC_PROFILE.scopes, - material: [ - { name: "client_email", required: true }, - { name: "private_key", required: true, secret: true }, - { name: "scope" }, - ], - }, - }, - ], - endpoints: [{ host: "chat.googleapis.com", port: 443 }], - binaries: ["/usr/local/bin/node", "/usr/bin/node"], - inference_capable: false, -}; - // Google Chat is the one channel shipping a profile per agent, so a sandbox must // pick exactly one. Hermes also needs pubsub on top of chat.bot: one token, both // scopes, because `:pull` 403s without it. @@ -119,6 +93,44 @@ const DISCORD_PROFILE_DOC = { inference_capable: false, }; +const GC_PROFILE_DOC = { + id: GC_PROFILE.profileId, + display_name: "Google Chat Bridge", + description: "Gateway-minted Google Chat bot token", + category: "agent", + credentials: [ + { + name: "access_token", + description: "Google Chat access token", + env_vars: [GC_PROFILE.credentialKey], + required: true, + auth_style: "bearer", + header_name: "Authorization", + query_param: "", + refresh: { + strategy: "google-service-account-jwt", + scopes: [...GC_PROFILE.scopes], + material: [ + { name: "client_email", description: "JWT issuer", required: true }, + { name: "private_key", description: "JWT key", required: true, secret: true }, + { name: "scope", description: "OAuth scopes" }, + ], + }, + }, + ], + endpoints: [ + { + host: "chat.googleapis.com", + port: 443, + protocol: "rest", + access: "read-write", + enforcement: "enforce", + }, + ], + binaries: ["/usr/local/bin/node", "/usr/bin/node"], + inference_capable: false, +}; + const STATIC_DEF = { name: "sbx-discord-bridge", providerType: DISCORD_PROFILE.profileId, @@ -219,7 +231,7 @@ describe("configureMessagingBridgeRefreshes", () => { const runOpenshell = vi.fn(); const result = configureMessagingBridgeRefreshes([], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [GC_PROFILE], @@ -229,55 +241,47 @@ describe("configureMessagingBridgeRefreshes", () => { }); it("fails closed when the secret is unavailable", () => { - const runOpenshell = vi.fn(); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { - runOpenshell, - redactFull, + runOpenshell: vi.fn(), + redact, getCredential: () => null, log: noLog, profiles: [GC_PROFILE], }); expect(result.ok).toBe(false); - expect(runOpenshell).not.toHaveBeenCalled(); }); it("fails closed when the service account JSON cannot be parsed", () => { - const runOpenshell = vi.fn(); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { - runOpenshell, - redactFull, + runOpenshell: vi.fn(), + redact, getCredential: () => "not json", log: noLog, profiles: [GC_PROFILE], }); expect(result.ok).toBe(false); - expect(runOpenshell).not.toHaveBeenCalled(); }); it("fails closed when client_email or private_key is missing", () => { - const runOpenshell = vi.fn(); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { - runOpenshell, - redactFull, + runOpenshell: vi.fn(), + redact, getCredential: () => JSON.stringify({ client_email: "x@y" }), log: noLog, profiles: [GC_PROFILE], }); expect(result.ok).toBe(false); - expect(runOpenshell).not.toHaveBeenCalled(); }); it("fails closed when client_email or private_key is blank", () => { - const runOpenshell = vi.fn(); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { - runOpenshell, - redactFull, + runOpenshell: vi.fn(), + redact, getCredential: () => JSON.stringify({ client_email: " ", private_key: "\n" }), log: noLog, profiles: [GC_PROFILE], }); expect(result.ok).toBe(false); - expect(runOpenshell).not.toHaveBeenCalled(); }); it("keeps private keys off argv while configuring refresh", () => { @@ -289,7 +293,7 @@ describe("configureMessagingBridgeRefreshes", () => { })); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [GC_PROFILE], @@ -321,7 +325,7 @@ describe("configureMessagingBridgeRefreshes", () => { const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [{ ...GC_PROFILE, scopes: GC_PUBSUB_SCOPES }], @@ -346,7 +350,7 @@ describe("configureMessagingBridgeRefreshes", () => { })); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [misconfigured], @@ -364,7 +368,7 @@ describe("configureMessagingBridgeRefreshes", () => { const runOpenshell = vi.fn(() => ({ status: 1, stderr: "gateway rejected the material" })); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [GC_PROFILE], @@ -373,41 +377,11 @@ describe("configureMessagingBridgeRefreshes", () => { expect(result.reason).toBeTruthy(); }); - it("fully redacts reflected source and generated refresh material", () => { - const clientEmail = "reflected-bot@example.invalid"; - const privateKey = "unique-private-key-material-for-redaction"; - const sourceSecret = JSON.stringify({ client_email: clientEmail, private_key: privateKey }); - const scope = GC_PROFILE.scopes.join(" "); - const log = vi.fn(); - const runOpenshell = vi.fn(() => ({ - status: 1, - stderr: `source=${sourceSecret} private_key=${privateKey}`, - stdout: `client_email=${clientEmail} scope=${scope}`, - })); - - const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { - runOpenshell, - redactFull, - getCredential: () => sourceSecret, - log, - profiles: [GC_PROFILE], - }); - - const diagnostic = `${log.mock.calls.flat().join("\n")} ${result.reason ?? ""}`; - expect(result.ok).toBe(false); - expect(diagnostic).not.toContain(sourceSecret); - expect(diagnostic).not.toContain(clientEmail); - expect(diagnostic).not.toContain(privateKey); - expect(diagnostic).not.toContain(scope); - expect(diagnostic).not.toContain(privateKey.slice(0, 4)); - expect(diagnostic).toContain(""); - }); - it("resolves the secret from the injected env too (parity)", () => { const runOpenshell = vi.fn(() => ({ status: 0, stdout: MINTED_STATUS_TABLE })); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => null, env: { [GC_PROFILE.sourceSecretEnv]: SA_JSON }, normalizeCredentialValue, @@ -425,7 +399,7 @@ describe("configureMessagingBridgeRefreshes", () => { })); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [GC_PROFILE], @@ -438,32 +412,6 @@ describe("configureMessagingBridgeRefreshes", () => { expect(statusArgs).toContain(GC_PROFILE.credentialKey); }); - it("waits through a configured status before accepting the refreshed token", () => { - const runOpenshell = vi - .fn() - .mockReturnValueOnce({ status: 0 }) - .mockReturnValueOnce({ status: 0, stdout: PENDING_STATUS_TABLE }) - .mockReturnValueOnce({ status: 0, stdout: MINTED_STATUS_TABLE }); - const sleep = vi.fn(); - - const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { - runOpenshell, - redactFull, - getCredential: () => SA_JSON, - log: noLog, - profiles: [GC_PROFILE], - sleep, - }); - - expect(result).toEqual({ ok: true }); - expect(runOpenshell.mock.calls.slice(1).map(([args]) => args.slice(0, 3))).toEqual([ - ["provider", "refresh", "status"], - ["provider", "refresh", "status"], - ]); - expect(sleep).toHaveBeenCalledOnce(); - expect(sleep).toHaveBeenCalledWith(3_000); - }); - it("fails closed when the gateway never mints the first token", () => { // Reporting success here would let onboarding create the sandbox while the // provider still holds the create-time sentinel: @@ -474,19 +422,16 @@ describe("configureMessagingBridgeRefreshes", () => { status: 0, stdout: PENDING_STATUS_TABLE, })); - const sleep = vi.fn(); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [GC_PROFILE], - sleep, + sleep: () => undefined, }); expect(result.ok).toBe(false); expect(result.reason).toContain("configured"); - expect(runOpenshell.mock.calls.filter((call) => call[0][2] === "status")).toHaveLength(50); - expect(sleep).toHaveBeenCalledTimes(49); }); it("rejects a refreshed row printed by a failed status command", () => { @@ -498,7 +443,7 @@ describe("configureMessagingBridgeRefreshes", () => { })); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [GC_PROFILE], @@ -517,7 +462,7 @@ describe("configureMessagingBridgeRefreshes", () => { }); const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [GC_PROFILE], @@ -525,9 +470,11 @@ describe("configureMessagingBridgeRefreshes", () => { now: () => clock, }); expect(result.ok).toBe(false); - // The configure command advances the clock once. Exactly five one-minute - // status probes then consume the five-minute polling deadline. - expect(runOpenshell.mock.calls.filter((call) => call[0][2] === "status")).toHaveLength(5); + // Six probes at a minute each cross the five-minute deadline well before + // the fifty-attempt cap. + expect( + runOpenshell.mock.calls.filter((call) => call[0][2] === "status").length, + ).toBeLessThan(10); }); it("bounds each status probe with a command timeout", () => { @@ -537,7 +484,7 @@ describe("configureMessagingBridgeRefreshes", () => { })); configureMessagingBridgeRefreshes([BRIDGE_DEF], { runOpenshell, - redactFull, + redact, getCredential: () => SA_JSON, log: noLog, profiles: [GC_PROFILE], @@ -546,24 +493,6 @@ describe("configureMessagingBridgeRefreshes", () => { const statusCall = runOpenshell.mock.calls.find((call) => call[0][2] === "status"); expect(statusCall?.[1]).toMatchObject({ timeout: 15_000 }); }); - - it("bounds refresh configuration and fails closed when the command times out", () => { - const runOpenshell = vi - .fn() - .mockReturnValueOnce({ status: null, stderr: "operation timed out" }); - - const result = configureMessagingBridgeRefreshes([BRIDGE_DEF], { - runOpenshell, - redactFull, - getCredential: () => SA_JSON, - log: noLog, - profiles: [GC_PROFILE], - }); - - expect(result).toEqual({ ok: false, reason: "operation timed out" }); - expect(runOpenshell).toHaveBeenCalledOnce(); - expect(runOpenshell.mock.calls[0]?.[1]).toMatchObject({ timeout: 30_000 }); - }); }); describe("refreshStatusForCredential", () => { @@ -585,10 +514,21 @@ describe("refreshStatusForCredential", () => { describe("ensureMessagingBridgeProfiles", () => { const baseDeps = () => ({ root: "/repo", + redact, log: noLog, exit: vi.fn(() => undefined as never), profiles: [GC_PROFILE], - }); + readFileSync: () => YAML.stringify(GC_PROFILE_DOC), + }); + const importAndExport = ( + exported: Record, + importResult: { status: number; stderr?: string } = { status: 0 }, + ) => + vi.fn((args: string[]) => + args.includes("import") + ? importResult + : { status: 0, stdout: JSON.stringify(exported) }, + ); it("does nothing when there is no bridge token def", () => { const runOpenshell = vi.fn(); @@ -596,356 +536,282 @@ describe("ensureMessagingBridgeProfiles", () => { expect(runOpenshell).not.toHaveBeenCalled(); }); - it("imports the profile from its co-located path when not yet registered", () => { - const runOpenshell = vi - .fn() - .mockReturnValueOnce({ status: 1, stderr: "provider profile not found" }) - .mockReturnValueOnce({ status: 0 }) - .mockReturnValueOnce({ status: 0, stdout: JSON.stringify(GC_PROFILE_DOC) }); + it("imports and validates the profile from its co-located path", () => { + const runOpenshell = importAndExport(GC_PROFILE_DOC); const exit = vi.fn(() => undefined as never); - ensureMessagingBridgeProfiles([BRIDGE_DEF], { - ...baseDeps(), - readFileSync: () => YAML.stringify(GC_PROFILE_DOC), - runOpenshell, - exit, - }); - const importCall = runOpenshell.mock.calls.find((call) => call[0].includes("import")); - expect(importCall?.[0].slice(0, 4)).toEqual(["provider", "profile", "import", "--file"]); - expect(importCall?.[0]).toContain(GC_PROFILE.profilePath); - expect(runOpenshell.mock.calls.map(([args]) => args)).toEqual([ - ["provider", "profile", "export", GC_PROFILE.profileId, "--output", "json"], - ["provider", "profile", "import", "--file", GC_PROFILE.profilePath], - ["provider", "profile", "export", GC_PROFILE.profileId, "--output", "json"], - ]); - expect(runOpenshell.mock.calls.map(([, options]) => options.timeout)).toEqual([ - 30_000, 30_000, 30_000, - ]); - expect(exit).not.toHaveBeenCalled(); - }); - it("validates without importing when the profile is already registered", () => { - // A fresh onboard registers bridge providers twice; the second pass must not - // re-import and trigger OpenShell's "already exists / import failed" output. - const runOpenshell = vi.fn((_args: string[], _opts: unknown) => ({ - status: 0, - stdout: JSON.stringify(GC_PROFILE_DOC), - })); - const exit = vi.fn(() => undefined as never); - ensureMessagingBridgeProfiles([BRIDGE_DEF], { - ...baseDeps(), - readFileSync: () => YAML.stringify(GC_PROFILE_DOC), - runOpenshell, - exit, - }); - expect(runOpenshell.mock.calls.some((call) => call[0].includes("import"))).toBe(false); - const exportCall = runOpenshell.mock.calls.find((call) => call[0].includes("export")); - expect(exportCall?.[0]).toEqual([ - "provider", - "profile", - "export", - GC_PROFILE.profileId, - "--output", - "json", + ensureMessagingBridgeProfiles([BRIDGE_DEF], { ...baseDeps(), runOpenshell, exit }); + + expect(runOpenshell.mock.calls.map((call) => call[0].slice(0, 3))).toEqual([ + ["provider", "profile", "import"], + ["provider", "profile", "export"], ]); - expect(exportCall?.[1]).toMatchObject({ suppressOutput: true, timeout: 30_000 }); + expect(runOpenshell.mock.calls[0]?.[0]).toContain(GC_PROFILE.profilePath); expect(exit).not.toHaveBeenCalled(); }); - it("accepts an existing static profile only when its credential boundary matches", () => { - const runOpenshell = vi.fn((_args: string[], _opts: unknown) => ({ - status: 0, - stdout: JSON.stringify(DISCORD_PROFILE_DOC), - })); + it("validates an exact profile returned after an already-exists import race", () => { + const runOpenshell = importAndExport(GC_PROFILE_DOC, { + status: 1, + stderr: "profile already exists", + }); const exit = vi.fn(() => undefined as never); - ensureMessagingBridgeProfiles([STATIC_DEF], { - ...baseDeps(), - profiles: [DISCORD_PROFILE], - readFileSync: () => YAML.stringify(DISCORD_PROFILE_DOC), - runOpenshell, - exit, - }); + ensureMessagingBridgeProfiles([BRIDGE_DEF], { ...baseDeps(), runOpenshell, exit }); - expect(runOpenshell).toHaveBeenCalledTimes(1); - expect(runOpenshell.mock.calls[0]?.[0]).toEqual([ - "provider", - "profile", - "export", - DISCORD_PROFILE.profileId, - "--output", - "json", - ]); - expect(runOpenshell.mock.calls[0]?.[1]).toMatchObject({ timeout: 30_000 }); + expect(runOpenshell).toHaveBeenCalledTimes(2); expect(exit).not.toHaveBeenCalled(); }); it.each([ - ["endpoint authority", { endpoints: [{ host: "gateway.discord.gg", port: 443 }] }], - ["binary authority", { binaries: ["/usr/bin/curl"] }], [ - "credential configuration", + "static endpoint authority", + STATIC_DEF, + DISCORD_PROFILE, + DISCORD_PROFILE_DOC, + { endpoints: [{ host: "gateway.discord.gg", port: 443 }] }, + ], + [ + "static binary authority", + STATIC_DEF, + DISCORD_PROFILE, + DISCORD_PROFILE_DOC, + { binaries: ["/usr/bin/curl"] }, + ], + [ + "static credential configuration", + STATIC_DEF, + DISCORD_PROFILE, + DISCORD_PROFILE_DOC, + { + credentials: [ + { ...DISCORD_PROFILE_DOC.credentials[0], header_name: "X-Discord-Token" }, + ], + }, + ], + [ + "refresh-enabled endpoint authority", + BRIDGE_DEF, + GC_PROFILE, + GC_PROFILE_DOC, + { endpoints: [{ ...GC_PROFILE_DOC.endpoints[0], host: "evil.invalid" }] }, + ], + [ + "refresh-enabled binary authority", + BRIDGE_DEF, + GC_PROFILE, + GC_PROFILE_DOC, + { binaries: ["/tmp/untrusted-node"] }, + ], + [ + "refresh-enabled credential configuration", + BRIDGE_DEF, + GC_PROFILE, + GC_PROFILE_DOC, + { + credentials: [ + { ...GC_PROFILE_DOC.credentials[0], header_name: "X-Google-Chat-Token" }, + ], + }, + ], + [ + "refresh-enabled refresh material", + BRIDGE_DEF, + GC_PROFILE, + GC_PROFILE_DOC, { credentials: [ { - ...DISCORD_PROFILE_DOC.credentials[0], - header_name: "X-Discord-Token", + ...GC_PROFILE_DOC.credentials[0], + refresh: { + ...GC_PROFILE_DOC.credentials[0].refresh, + material: [{ name: "attacker_material", required: true, secret: true }], + }, }, ], }, ], - ])("rejects an existing static profile with different %s", (_label, override) => { - const exported = { ...DISCORD_PROFILE_DOC, ...override }; - const runOpenshell = vi.fn((_args: string[], _opts: unknown) => ({ - status: 0, - stdout: JSON.stringify(exported), - })); - const exit = vi.fn(() => undefined as never); + ] as const)( + "rejects an already-registered profile with different %s", + (_label, tokenDef, profile, checkedIn, override) => { + const runOpenshell = importAndExport( + { ...checkedIn, ...override }, + { status: 1, stderr: "profile already exists" }, + ); + const exit = vi.fn(() => undefined as never); - ensureMessagingBridgeProfiles([STATIC_DEF], { - ...baseDeps(), - profiles: [DISCORD_PROFILE], - readFileSync: () => YAML.stringify(DISCORD_PROFILE_DOC), - runOpenshell, - exit, - }); + ensureMessagingBridgeProfiles([tokenDef], { + ...baseDeps(), + profiles: [profile], + readFileSync: () => YAML.stringify(checkedIn), + runOpenshell, + exit, + }); - expect(exit).toHaveBeenCalledWith(1); - expect(runOpenshell.mock.calls.some((call) => call[0].includes("import"))).toBe(false); - expect(runOpenshell).toHaveBeenCalledTimes(1); - }); + expect(runOpenshell).toHaveBeenCalledTimes(2); + expect(exit).toHaveBeenCalledWith(1); + }, + ); - it("rejects a mismatched static profile that wins an import race", () => { - const runOpenshell = vi - .fn() - .mockReturnValueOnce({ status: 1, stderr: "provider profile not found" }) - .mockReturnValueOnce({ status: 1, stderr: "profile already exists" }) - .mockReturnValueOnce({ - status: 0, - stdout: JSON.stringify({ ...DISCORD_PROFILE_DOC, binaries: ["/usr/bin/curl"] }), - }); + it("rejects a changed boundary after a successful import", () => { + const runOpenshell = importAndExport({ + ...GC_PROFILE_DOC, + endpoints: [{ ...GC_PROFILE_DOC.endpoints[0], host: "evil.invalid" }], + }); const exit = vi.fn(() => undefined as never); - ensureMessagingBridgeProfiles([STATIC_DEF], { - ...baseDeps(), - profiles: [DISCORD_PROFILE], - readFileSync: () => YAML.stringify(DISCORD_PROFILE_DOC), - runOpenshell, - exit, - }); + ensureMessagingBridgeProfiles([BRIDGE_DEF], { ...baseDeps(), runOpenshell, exit }); + expect(runOpenshell).toHaveBeenCalledTimes(2); expect(exit).toHaveBeenCalledWith(1); - expect(runOpenshell).toHaveBeenCalledTimes(3); }); - it.each([ - { - condition: "is already registered", - results: [ - { - status: 0, - stdout: JSON.stringify({ - ...GC_PROFILE_DOC, - endpoints: [...GC_PROFILE_DOC.endpoints, { host: "untrusted.invalid", port: 443 }], - }), - }, - ], - }, - { - condition: "wins an import race", - results: [ - { status: 1, stderr: "provider profile not found" }, - { status: 1, stderr: "profile already exists" }, - { - status: 0, - stdout: JSON.stringify({ - ...GC_PROFILE_DOC, - binaries: [...GC_PROFILE_DOC.binaries, "/usr/bin/curl"], - }), - }, - ], - }, - ])("rejects a mismatched refreshing profile that $condition", ({ results }) => { - const queuedResults = [...results]; - const runOpenshell = vi.fn(() => queuedResults.shift() ?? { status: 1 }); + it("fails closed when post-import export is unavailable", () => { + const runOpenshell = vi + .fn() + .mockReturnValueOnce({ status: 0 }) + .mockReturnValueOnce({ status: 1, stderr: "gateway unavailable" }); const exit = vi.fn(() => undefined as never); - ensureMessagingBridgeProfiles([BRIDGE_DEF], { - ...baseDeps(), - readFileSync: () => YAML.stringify(GC_PROFILE_DOC), - runOpenshell, - exit, - }); + ensureMessagingBridgeProfiles([BRIDGE_DEF], { ...baseDeps(), runOpenshell, exit }); expect(exit).toHaveBeenCalledWith(1); - expect(runOpenshell).toHaveBeenCalledTimes(results.length); }); - it("accepts a matching refreshing profile that wins an import race", () => { - const runOpenshell = vi - .fn() - .mockReturnValueOnce({ status: 1, stderr: "provider profile not found" }) - .mockReturnValueOnce({ status: 1, stderr: "profile already exists" }) - .mockReturnValueOnce({ status: 0, stdout: JSON.stringify(GC_PROFILE_DOC) }); + it("exits when profile import fails", () => { + const runOpenshell = vi.fn(() => ({ status: 1, stderr: "connection refused" })); const exit = vi.fn(() => undefined as never); - ensureMessagingBridgeProfiles([BRIDGE_DEF], { - ...baseDeps(), - readFileSync: () => YAML.stringify(GC_PROFILE_DOC), - runOpenshell, - exit, - }); - expect(exit).not.toHaveBeenCalled(); - expect(runOpenshell).toHaveBeenCalledTimes(3); - }); - it.each(["connection refused", "authentication failed"])( - "does not import when the profile probe fails with %s", - (diagnostic) => { - const runOpenshell = vi.fn((_args: string[], _options: unknown) => ({ - status: 1, - stderr: diagnostic, - })); - const exit = vi.fn(() => undefined as never); - ensureMessagingBridgeProfiles([BRIDGE_DEF], { - ...baseDeps(), - readFileSync: () => YAML.stringify(GC_PROFILE_DOC), - runOpenshell, - exit, - }); - expect(exit).toHaveBeenCalledWith(1); - expect(runOpenshell).toHaveBeenCalledTimes(1); - expect(runOpenshell.mock.calls.some((call) => call[0].includes("import"))).toBe(false); - expect(runOpenshell.mock.calls[0]?.[1]).toMatchObject({ timeout: 30_000 }); - }, - ); + ensureMessagingBridgeProfiles([BRIDGE_DEF], { ...baseDeps(), runOpenshell, exit }); - it("exits when a confirmed-missing profile cannot be imported", () => { - const runOpenshell = vi - .fn() - .mockReturnValueOnce({ status: 1, stderr: "provider profile not found" }) - .mockReturnValueOnce({ status: 1, stderr: "connection refused" }); - const exit = vi.fn(() => undefined as never); - ensureMessagingBridgeProfiles([BRIDGE_DEF], { - ...baseDeps(), - readFileSync: () => YAML.stringify(GC_PROFILE_DOC), - runOpenshell, - exit, - }); + expect(runOpenshell).toHaveBeenCalledTimes(1); expect(exit).toHaveBeenCalledWith(1); - expect(runOpenshell).toHaveBeenCalledTimes(2); - expect(runOpenshell.mock.calls.map(([, options]) => options.timeout)).toEqual([30_000, 30_000]); }); }); -describe("matchesRegisteredMessagingBridgeProfile", () => { - it("accepts only the checked-in static credential boundary", () => { - const runOpenshell = vi.fn(() => ({ - status: 0, - stdout: JSON.stringify(DISCORD_PROFILE_DOC), - })); +describe("inspectRegisteredStaticMessagingProfile", () => { + it("distinguishes static profile drift from an unavailable gateway inspection", () => { + const deps = { + root: "/repo", + profiles: [DISCORD_PROFILE], + readFileSync: () => YAML.stringify(DISCORD_PROFILE_DOC), + }; expect( - matchesRegisteredMessagingBridgeProfile(DISCORD_PROFILE.profileId, { - root: "/repo", - profiles: [DISCORD_PROFILE], - readFileSync: () => YAML.stringify(DISCORD_PROFILE_DOC), - runOpenshell, + inspectRegisteredStaticMessagingProfile(DISCORD_PROFILE.profileId, { + ...deps, + runOpenshell: () => ({ status: 1, stderr: "gateway unavailable" }), }), - ).toBe(true); - expect(runOpenshell).toHaveBeenCalledWith( - ["provider", "profile", "export", DISCORD_PROFILE.profileId, "--output", "json"], - expect.objectContaining({ suppressOutput: true, timeout: 30_000 }), - ); - }); - - it("rejects a registered static profile with endpoint authority", () => { - const runOpenshell = vi.fn(() => ({ - status: 0, - stdout: JSON.stringify({ - ...DISCORD_PROFILE_DOC, - endpoints: [{ host: "gateway.discord.gg", port: 443 }], + ).toEqual({ kind: "indeterminate" }); + expect( + inspectRegisteredStaticMessagingProfile(DISCORD_PROFILE.profileId, { + ...deps, + runOpenshell: () => { + throw new Error("transport closed"); + }, }), - })); - + ).toEqual({ kind: "indeterminate" }); expect( - matchesRegisteredMessagingBridgeProfile(DISCORD_PROFILE.profileId, { - root: "/repo", - profiles: [DISCORD_PROFILE], - readFileSync: () => YAML.stringify(DISCORD_PROFILE_DOC), - runOpenshell, + inspectRegisteredStaticMessagingProfile(DISCORD_PROFILE.profileId, { + ...deps, + runOpenshell: () => ({ status: 0, stdout: "" }), }), - ).toBe(false); - }); - - it("rejects a registered refreshing profile with altered refresh authority", () => { - const runOpenshell = vi.fn(() => ({ - status: 0, - stdout: JSON.stringify({ - ...GC_PROFILE_DOC, - credentials: [ - { - ...GC_PROFILE_DOC.credentials[0], - refresh: { - ...GC_PROFILE_DOC.credentials[0].refresh, - scopes: [...GC_PROFILE.scopes, "https://www.googleapis.com/auth/cloud-platform"], - }, - }, - ], + ).toEqual({ kind: "indeterminate" }); + expect( + inspectRegisteredStaticMessagingProfile(DISCORD_PROFILE.profileId, { + ...deps, + runOpenshell: () => ({ status: 0, stdout: "not-json" }), }), - })); - + ).toEqual({ kind: "indeterminate" }); expect( - matchesRegisteredMessagingBridgeProfile(GC_PROFILE.profileId, { - root: "/repo", - profiles: [GC_PROFILE], - readFileSync: () => YAML.stringify(GC_PROFILE_DOC), - runOpenshell, + inspectRegisteredStaticMessagingProfile(DISCORD_PROFILE.profileId, { + ...deps, + runOpenshell: () => ({ + status: 0, + stdout: JSON.stringify({ ...DISCORD_PROFILE_DOC, binaries: ["/usr/bin/curl"] }), + }), }), - ).toBe(false); - }); - - it("does not apply the static-profile check to other provider types", () => { - const runOpenshell = vi.fn(); - + ).toEqual({ kind: "collision" }); expect( - matchesRegisteredMessagingBridgeProfile("generic", { - root: "/repo", - profiles: [DISCORD_PROFILE], - runOpenshell, + inspectRegisteredStaticMessagingProfile(DISCORD_PROFILE.profileId, { + ...deps, + readFileSync: () => + YAML.stringify({ + ...DISCORD_PROFILE_DOC, + credentials: DISCORD_PROFILE_DOC.credentials.map((credential) => ({ + ...credential, + required: "yes", + })), + }), + runOpenshell: () => ({ + status: 0, + stdout: JSON.stringify(DISCORD_PROFILE_DOC), + }), }), - ).toBeNull(); - expect(runOpenshell).not.toHaveBeenCalled(); + ).toEqual({ kind: "indeterminate" }); + expect( + inspectRegisteredStaticMessagingProfile(DISCORD_PROFILE.profileId, { + ...deps, + readFileSync: () => { + throw new Error("profile unreadable"); + }, + runOpenshell: () => ({ + status: 0, + stdout: JSON.stringify(DISCORD_PROFILE_DOC), + }), + }), + ).toEqual({ kind: "indeterminate" }); }); }); describe("listMessagingBridgeProfiles", () => { - it("discovers a co-located bridge profile from injected manifests and YAML", () => { - const manifest: ChannelManifest = { - schemaVersion: 1, - id: GC_PROFILE.channelId, - displayName: "Fixture chat", - supportedAgents: [GC_PROFILE.agent], - auth: { mode: "token-paste" }, - inputs: [ - { - id: "serviceAccount", - kind: "secret", - required: true, - envKey: GC_PROFILE.sourceSecretEnv, - }, - ], - credentials: [], - render: [], - hooks: [], - }; + const manifest = { + id: "fixture-chat", + supportedAgents: ["openclaw"], + inputs: [{ kind: "secret", required: true, envKey: "FIXTURE_SERVICE_ACCOUNT" }], + } as unknown as ChannelManifest; + const profileDoc = { + ...GC_PROFILE_DOC, + id: "fixture-chat-bridge", + credentials: [ + { + ...GC_PROFILE_DOC.credentials[0], + env_vars: ["FIXTURE_ACCESS_TOKEN"], + }, + ], + }; + + it("discovers a valid synthetic channel profile", () => { + const profiles = listMessagingBridgeProfiles({ + root: "/synthetic", + manifests: [manifest], + existsSync: () => true, + readFileSync: () => YAML.stringify(profileDoc), + }); + + expect(profiles).toEqual([ + expect.objectContaining({ + channelId: "fixture-chat", + agent: "openclaw", + profileId: "fixture-chat-bridge", + credentialKey: "FIXTURE_ACCESS_TOKEN", + strategy: "google-service-account-jwt", + sourceSecretEnv: "FIXTURE_SERVICE_ACCOUNT", + }), + ]); + expect(profiles[0]?.secretMaterialKeys).toContain("private_key"); + }); + it("ignores absent and malformed synthetic profiles", () => { + const dependencies = { root: "/synthetic", manifests: [manifest] }; + expect(listMessagingBridgeProfiles({ ...dependencies, existsSync: () => false })).toEqual([]); expect( listMessagingBridgeProfiles({ - root: "/repo", - manifests: [manifest], + ...dependencies, existsSync: () => true, - readFileSync: () => YAML.stringify(GC_PROFILE_DOC), + readFileSync: () => "id: malformed", }), - ).toEqual([GC_PROFILE]); + ).toEqual([]); }); }); diff --git a/src/lib/onboard/messaging-bridge-provider.ts b/src/lib/onboard/messaging-bridge-provider.ts index 7872645959c..d6fa31b8d5d 100644 --- a/src/lib/onboard/messaging-bridge-provider.ts +++ b/src/lib/onboard/messaging-bridge-provider.ts @@ -19,12 +19,12 @@ import fs from "node:fs"; import path from "node:path"; import YAML from "yaml"; -import { OPENSHELL_OPERATION_TIMEOUT_MS } from "../adapters/openshell/provider-command"; +import { importCliOpenShellProviderProfile } from "../adapters/openshell/provider-adapter-cli"; import { - exportedProviderProfileMatchesContract, + compareExportedProviderProfileWithContract, parseCheckedInProviderProfileContract, } from "../adapters/openshell/provider-profile"; -import { registerCheckedInProviderProfile } from "../adapters/openshell/provider-profile-registration"; +import { selectedOpenShellGateway } from "../adapters/openshell/sandbox-observer"; import { compactText } from "../core/url-utils"; import { createBuiltInChannelManifestRegistry } from "../messaging/channels"; import type { @@ -33,7 +33,6 @@ import type { MessagingAgentId, } from "../messaging/manifest"; import { ROOT } from "../state/paths"; -import { sleepMs, waitUntil } from "./readiness-wait"; // Create-time credential sentinel: the real value is minted by // `provider refresh configure`; this only has to be non-empty so the provider is @@ -114,22 +113,29 @@ export interface CollectMessagingBridgeTokenDefsInput extends MessagingBridgeSec export interface EnsureMessagingBridgeProfilesDeps { readonly root: string; readonly runOpenshell: RunOpenshell; + readonly redact: (input: string) => string; readonly log?: (message?: string) => void; readonly exit?: (code?: number) => never; readonly profiles?: readonly MessagingBridgeProfile[]; readonly readFileSync?: (file: string) => string; } -export interface MatchRegisteredMessagingBridgeProfileDeps { +export interface MatchRegisteredStaticMessagingProfileDeps { readonly root: string; readonly runOpenshell: RunOpenshell; readonly profiles?: readonly MessagingBridgeProfile[]; readonly readFileSync?: (file: string) => string; } +export type RegisteredStaticMessagingProfileInspection = + | { readonly kind: "collision" } + | { readonly kind: "exact" } + | { readonly kind: "indeterminate" } + | { readonly kind: "not-static" }; + export interface ConfigureMessagingBridgeRefreshesDeps extends MessagingBridgeSecretResolveDeps { readonly runOpenshell: RunOpenshell; - readonly redactFull: (input: string) => string; + readonly redact: (input: string) => string; readonly log?: (message?: string) => void; readonly profiles?: readonly MessagingBridgeProfile[]; /** Injected for tests; defaults to a synchronous wait. */ @@ -150,67 +156,62 @@ function bufferOrStringToText(value: string | Buffer | null | undefined): string return ""; } -function redactRefreshDiagnostic( - input: string, - sourceSecret: string, - materialValues: readonly string[], - redactFull: (input: string) => string, -): string { - let redacted = input; - const exactValues = Array.from(new Set([sourceSecret, ...materialValues])).sort( - (left, right) => right.length - left.length, - ); - for (const value of exactValues) { - if (value) redacted = redacted.replaceAll(value, ""); +function checkedInProfileContract( + profile: MessagingBridgeProfile, + readFileSync: (file: string) => string, +): ReturnType { + try { + const expected = parseCheckedInProviderProfileContract(readFileSync(profile.profilePath)); + return expected?.profileId === profile.profileId ? expected : null; + } catch { + return null; } - return redactFull(redacted); } -function profileMatchesCheckedInBoundary( +function staticProfileMatchesCheckedInBoundary( profile: MessagingBridgeProfile, exported: string, readFileSync: (file: string) => string, -): boolean { - try { - const expected = parseCheckedInProviderProfileContract(readFileSync(profile.profilePath)); - return ( - expected !== null && - expected.profileId === profile.profileId && - (profile.strategy !== null || - (expected.boundary.endpoints.length === 0 && - expected.boundary.binaries.length === 0 && - expected.boundary.inference_capable === false)) && - exportedProviderProfileMatchesContract(exported, expected) - ); - } catch { - return false; +): boolean | null { + const expected = checkedInProfileContract(profile, readFileSync); + if ( + expected === null || + expected.boundary.endpoints.length !== 0 || + expected.boundary.binaries.length !== 0 || + expected.boundary.inference_capable !== false + ) { + return null; } + return compareExportedProviderProfileWithContract(exported, expected); } -/** Compare a registered bridge profile with its checked-in credential boundary. */ -export function matchesRegisteredMessagingBridgeProfile( +/** Distinguish a checked-in static profile from drift and gateway inspection failure. */ +export function inspectRegisteredStaticMessagingProfile( providerType: string, - deps: MatchRegisteredMessagingBridgeProfileDeps, -): boolean | null { + deps: MatchRegisteredStaticMessagingProfileDeps, +): RegisteredStaticMessagingProfileInspection { const profile = (deps.profiles ?? listMessagingBridgeProfiles({ root: deps.root })).find( - (candidate) => candidate.profileId === providerType, - ); - if (!profile) return null; - const exported = deps.runOpenshell( - ["provider", "profile", "export", profile.profileId, "--output", "json"], - { - ignoreError: true, - suppressOutput: true, - stdio: ["ignore", "pipe", "pipe"], - timeout: OPENSHELL_OPERATION_TIMEOUT_MS, - }, - ); - if (exported.status !== 0) return false; - return profileMatchesCheckedInBoundary( - profile, - bufferOrStringToText(exported.stdout), - deps.readFileSync ?? ((file: string) => fs.readFileSync(file, "utf-8")), + (candidate) => candidate.profileId === providerType && candidate.strategy === null, ); + if (!profile) return { kind: "not-static" }; + try { + const exported = deps.runOpenshell( + ["provider", "profile", "export", profile.profileId, "--output", "json"], + { ignoreError: true, suppressOutput: true, stdio: ["ignore", "pipe", "pipe"] }, + ); + if (exported.status !== 0) return { kind: "indeterminate" }; + const matches = staticProfileMatchesCheckedInBoundary( + profile, + bufferOrStringToText(exported.stdout), + deps.readFileSync ?? ((file: string) => fs.readFileSync(file, "utf-8")), + ); + if (matches === null) return { kind: "indeterminate" }; + return { + kind: matches ? "exact" : "collision", + }; + } catch { + return { kind: "indeterminate" }; + } } function isSafeChannelId(value: string): boolean { @@ -356,9 +357,8 @@ function bridgeProviderNameFor(sandboxName: string, channelId: string): string { /** * Build the messaging token definitions for every enabled bridge channel whose * source secret was captured. Mirrors how the Brave provider is pushed in - * messaging-prep: the value is a non-empty sentinel used only to create a - * missing provider. An exact existing provider keeps its working credential - * until refresh succeeds. The real material is supplied separately by + * messaging-prep: the value is a non-empty sentinel (overwritten by the first + * refresh) and the real material is supplied separately by * {@link configureMessagingBridgeRefreshes}. */ export function collectMessagingBridgeTokenDefs( @@ -451,37 +451,25 @@ export function ensureMessagingBridgeProfiles( const errorLog = deps.log ?? console.error; const exit = deps.exit ?? ((code?: number) => process.exit(code)); + const readFileSync = deps.readFileSync ?? ((file: string) => fs.readFileSync(file, "utf-8")); + for (const profile of active) { - const result = registerCheckedInProviderProfile({ - profilePath: profile.profilePath, - runOpenshell: deps.runOpenshell, - readProfileFile: deps.readFileSync, - }); + // The caller supplies a runner already scoped to the authoritative gateway; + // selected targeting avoids adding a second, conflicting gateway flag. + const result = importCliOpenShellProviderProfile( + { target: selectedOpenShellGateway(), profilePath: profile.profilePath }, + { run: deps.runOpenshell, readProfileFile: readFileSync }, + ); if (result.ok) continue; - if (result.error.kind === "command" && result.error.reason === "profile_incompatible") { - const contract = - profile.strategy === null - ? `endpointless ${profile.channelId} credential contract` - : `checked-in ${profile.channelId} credential boundary`; - errorLog( - `\n ✗ OpenShell provider profile '${profile.profileId}' does not match NemoClaw's ${contract}.`, - ); - errorLog(" Remove the conflicting profile and re-run onboarding."); - exit(1); - return; - } - - errorLog(`\n ✗ Failed to register the ${profile.channelId} provider profile with OpenShell.`); + errorLog( + `\n ✗ Failed to register the ${profile.channelId} provider profile with OpenShell.`, + ); errorLog(` ${result.error.message}`); - errorLog(" Inspect the preceding OpenShell error."); - if (result.error.kind === "schema") { - errorLog(" Update NemoClaw and its managed OpenShell with `nemoclaw update`."); - } else if (result.error.kind === "validation") { - errorLog(" Restore the checked-in provider profile, then retry."); - } else { - errorLog(" Confirm OpenShell is available and authorized, then retry."); - } - errorLog(" Then re-run onboarding."); + errorLog( + result.error.kind === "command" && result.error.reason === "profile_incompatible" + ? " Remove the conflicting profile and re-run onboarding." + : " Update OpenShell with scripts/install-openshell.sh and re-run onboarding.", + ); exit(1); return; } @@ -566,52 +554,43 @@ export function refreshStatusForCredential(text: string, credentialKey: string): return keyIndex < 0 ? "" : (columns[keyIndex + 2] ?? ""); } +function sleepSync(milliseconds: number): void { + // Vitest sets process.env.VITEST, so the poll loop costs no wall-clock in tests. + if (process.env.VITEST === "true" || process.env.NEMOCLAW_TEST_NO_SLEEP === "1") return; + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, milliseconds); +} + function waitForMintedBridgeCredential( providerName: string, credentialKey: string, deps: ConfigureMessagingBridgeRefreshesDeps, ): MessagingBridgeRefreshResult { + const sleep = deps.sleep ?? sleepSync; const now = deps.now ?? (() => Date.now()); - const sleep = - deps.sleep ?? - (process.env.VITEST === "true" || process.env.NEMOCLAW_TEST_NO_SLEEP === "1" - ? () => undefined - : sleepMs); // The mint runs on the gateway's own sweep, so this can sit for a minute. (deps.log ?? console.error)(` Waiting for the gateway to mint ${credentialKey}…`); const deadline = now() + BRIDGE_MINT_DEADLINE_MS; let status = ""; - const minted = waitUntil( - () => { - const result = deps.runOpenshell( - ["provider", "refresh", "status", providerName, "--credential-key", credentialKey], - // suppressOutput: the runner re-emits piped child output; without it every - // poll reprints the whole status table into the onboarding transcript. - { - ignoreError: true, - stdio: ["ignore", "pipe", "pipe"], - suppressOutput: true, - timeout: BRIDGE_MINT_STATUS_TIMEOUT_MS, - }, - ); - // A nonzero probe can still print a stale table; only trust a clean read. - status = - result.status === 0 - ? refreshStatusForCredential(bufferOrStringToText(result.stdout), credentialKey) - : ""; - return status === BRIDGE_MINT_STATUS_REFRESHED; - }, - { - deadlineMs: deadline, - initialIntervalMs: BRIDGE_MINT_POLL_INTERVAL_MS, - maxIntervalMs: BRIDGE_MINT_POLL_INTERVAL_MS, - backoffFactor: 1, - maxAttempts: BRIDGE_MINT_POLL_ATTEMPTS, - now, - sleep, - }, - ); - if (minted) return { ok: true }; + for (let attempt = 0; attempt < BRIDGE_MINT_POLL_ATTEMPTS && now() < deadline; attempt += 1) { + const result = deps.runOpenshell( + ["provider", "refresh", "status", providerName, "--credential-key", credentialKey], + // suppressOutput: the runner re-emits piped child output; without it every + // poll reprints the whole status table into the onboarding transcript. + { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + suppressOutput: true, + timeout: BRIDGE_MINT_STATUS_TIMEOUT_MS, + }, + ); + // A nonzero probe can still print a stale table; only trust a clean read. + status = + result.status === 0 + ? refreshStatusForCredential(bufferOrStringToText(result.stdout), credentialKey) + : ""; + if (status === BRIDGE_MINT_STATUS_REFRESHED) return { ok: true }; + sleep(BRIDGE_MINT_POLL_INTERVAL_MS); + } return { ok: false, reason: `gateway token minting did not complete for '${providerName}' (last status '${status || "unknown"}')`, @@ -674,72 +653,35 @@ export function configureMessagingBridgeRefreshes( } materialArgs.push("--material", `${key}=${value}`); } - let result; - try { - result = deps.runOpenshell( - [ - "provider", - "refresh", - "configure", - "--credential-key", - profile.credentialKey, - "--strategy", - profile.strategy, - ...materialArgs, - bridge.name, - ], - { - env: secretMaterialEnv, - ignoreError: true, - stdio: ["ignore", "pipe", "pipe"], - timeout: OPENSHELL_OPERATION_TIMEOUT_MS, - }, - ); - } catch (error) { - const diagnostic = compactText( - redactRefreshDiagnostic( - error instanceof Error ? error.message : String(error), - secret, - built.material.map(({ value }) => value), - deps.redactFull, - ), - ); - warn(`\n ✗ ${profile.channelId} bridge: gateway refresh configuration failed.`); - if (diagnostic) warn(` ${diagnostic.slice(0, 500)}`); - return { ok: false, reason: diagnostic || "gateway refresh configuration failed" }; - } + const result = deps.runOpenshell( + [ + "provider", + "refresh", + "configure", + "--credential-key", + profile.credentialKey, + "--strategy", + profile.strategy, + ...materialArgs, + bridge.name, + ], + { + env: secretMaterialEnv, + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }, + ); if (result.status === 0) { - let minted; - try { - minted = waitForMintedBridgeCredential(bridge.name, profile.credentialKey, deps); - } catch (error) { - const diagnostic = compactText( - redactRefreshDiagnostic( - error instanceof Error ? error.message : String(error), - secret, - built.material.map(({ value }) => value), - deps.redactFull, - ), - ); - warn(`\n ✗ ${profile.channelId} bridge: gateway refresh status inspection failed.`); - if (diagnostic) warn(` ${diagnostic.slice(0, 500)}`); - return { ok: false, reason: diagnostic || "gateway refresh status inspection failed" }; - } + const minted = waitForMintedBridgeCredential(bridge.name, profile.credentialKey, deps); if (minted.ok) continue; warn(`\n ✗ ${profile.channelId} bridge: ${minted.reason}.`); warn(" Outbound replies for this channel will not authenticate until this is resolved."); return minted; } - // Redact exact source/generated material as well as known secret shapes - // before logging. The diagnostic is bounded only after redaction. + // Redact before logging — never echo secret material. const diagnostic = compactText( - redactRefreshDiagnostic( - `${bufferOrStringToText(result.stderr)} ${bufferOrStringToText(result.stdout)}`, - secret, - built.material.map(({ value }) => value), - deps.redactFull, - ), + deps.redact(`${bufferOrStringToText(result.stderr)} ${bufferOrStringToText(result.stdout)}`), ); warn( `\n ✗ ${profile.channelId} bridge: failed to configure gateway token minting for '${bridge.name}'.`, diff --git a/src/lib/onboard/messaging-channel-setup.ts b/src/lib/onboard/messaging-channel-setup.ts index bc6a61a7045..c70c24f763c 100644 --- a/src/lib/onboard/messaging-channel-setup.ts +++ b/src/lib/onboard/messaging-channel-setup.ts @@ -122,8 +122,10 @@ export function detectMessagingChannelsFromEnv(agent: AgentDefinition | null = n * when the current selection omits it and the environment no longer configures * it under the same manifest input rules as * {@link detectMessagingChannelsFromEnv}, so a channel that still has some but - * not all of its required inputs also qualifies. `handlePoliciesState` then - * drops that channel's network-egress preset instead of carrying it forward. + * not all of its required inputs also qualifies. Reuse callers include channels + * backed by matching durable gateway providers in `selectedChannels`; process + * input absence alone does not opt out those channels. `handlePoliciesState` + * then drops only confirmed removals from the network-egress presets. * A channel enrolled inside the sandbox (`in-sandbox-qr`) never qualifies: the * host environment holds no value that reports whether the channel is still * paired. {@link resolveMessagingManifestSeed} exempts the same channels when diff --git a/src/lib/onboard/messaging-plan-session.test.ts b/src/lib/onboard/messaging-plan-session.test.ts new file mode 100644 index 00000000000..3a43b26af70 --- /dev/null +++ b/src/lib/onboard/messaging-plan-session.test.ts @@ -0,0 +1,35 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { makeMessagingPlan } from "../../../test/helpers/messaging-plan-fixtures"; +import { messagingChannelsWithReusableGatewayCredentials } from "./messaging-plan-session"; + +describe("messagingChannelsWithReusableGatewayCredentials", () => { + it.each([ + ["openclaw", "google-chat-bridge"], + ["hermes", "google-chat-hermes-bridge"], + ] as const)( + "reuses active Google Chat for %s only while its gateway-minted bridge provider matches", + (agent, providerType) => { + const plan = makeMessagingPlan({ channels: ["googlechat"], agent }); + const matches = vi.fn( + (name: string, type: string, credentialEnv: string) => + name === "my-assistant-googlechat-bridge" && + type === providerType && + credentialEnv === "GOOGLE_CHAT_ACCESS_TOKEN", + ); + + expect(messagingChannelsWithReusableGatewayCredentials(plan, matches)).toEqual([ + "googlechat", + ]); + expect(matches).toHaveBeenCalledExactlyOnceWith( + "my-assistant-googlechat-bridge", + providerType, + "GOOGLE_CHAT_ACCESS_TOKEN", + ); + expect(messagingChannelsWithReusableGatewayCredentials(plan, () => false)).toEqual([]); + }, + ); +}); diff --git a/src/lib/onboard/messaging-plan-session.ts b/src/lib/onboard/messaging-plan-session.ts index fe1c99e4879..23ae807f2ff 100644 --- a/src/lib/onboard/messaging-plan-session.ts +++ b/src/lib/onboard/messaging-plan-session.ts @@ -2,7 +2,16 @@ // SPDX-License-Identifier: Apache-2.0 import type { SandboxMessagingPlan } from "../messaging/manifest"; -import { getConfiguredChannelIdsFromPlan } from "../messaging/plan-validation"; +import { MESSAGING_CREDENTIAL_PROVIDER_TYPE } from "../messaging/provider-profile"; +import { + getActiveChannelIdsFromPlan, + getConfiguredChannelIdsFromPlan, +} from "../messaging/plan-validation"; +import { + bridgeProviderNamesForChannel, + messagingBridgeProfilesForAgent, + staticMessagingProviderTypeForChannel, +} from "./messaging-bridge-provider"; export { getActiveChannelIdsFromPlan as getActiveChannelsFromPlan, @@ -11,6 +20,50 @@ export { parseSandboxMessagingPlan, } from "../messaging/plan-validation"; +export type MessagingGatewayCredentialMatcher = ( + name: string, + type: string, + credentialEnv: string, +) => boolean; + +/** Keep active channels only while every gateway credential provider still matches. */ +export function messagingChannelsWithReusableGatewayCredentials( + plan: SandboxMessagingPlan | null | undefined, + providerMatchesGatewayCredential: MessagingGatewayCredentialMatcher, +): string[] { + if (!plan) return []; + const bridgeProfiles = messagingBridgeProfilesForAgent(plan.agent); + return getActiveChannelIdsFromPlan(plan).filter((channelId) => { + const bindings = plan.credentialBindings.filter((binding) => binding.channelId === channelId); + if (bindings.length > 0) { + return bindings.every((binding) => + providerMatchesGatewayCredential( + binding.providerName, + staticMessagingProviderTypeForChannel(binding.channelId, plan.agent) ?? + MESSAGING_CREDENTIAL_PROVIDER_TYPE, + binding.providerEnvKey, + ), + ); + } + + const gatewayBridgeProviders = bridgeProfiles + .filter((profile) => profile.channelId === channelId && profile.strategy !== null) + .flatMap((profile) => + bridgeProviderNamesForChannel(plan.sandboxName, channelId, [profile]).map((name) => ({ + name, + type: profile.profileId, + credentialEnv: profile.credentialKey, + })), + ); + return ( + gatewayBridgeProviders.length > 0 && + gatewayBridgeProviders.every(({ name, type, credentialEnv }) => + providerMatchesGatewayCredential(name, type, credentialEnv), + ) + ); + }); +} + /** Derive configured channel IDs from a plan. */ export function getChannelsFromPlan( plan: SandboxMessagingPlan | null | undefined, diff --git a/src/lib/onboard/providers.test.ts b/src/lib/onboard/providers.test.ts index 86935e42a70..bc40039e764 100644 --- a/src/lib/onboard/providers.test.ts +++ b/src/lib/onboard/providers.test.ts @@ -49,31 +49,6 @@ const MESSAGING_ENDPOINTLESS_PROFILE_EXPORT = JSON.stringify({ inference_capable: false, }); -const BRAVE_PROFILE_EXPORT = JSON.stringify({ - id: "brave", - credentials: [ - { - name: "api_key", - env_vars: ["BRAVE_API_KEY"], - required: true, - auth_style: "header", - header_name: "x-subscription-token", - query_param: "", - }, - ], - endpoints: [ - { - host: "api.search.brave.com", - port: 443, - protocol: "rest", - access: "read-write", - enforcement: "enforce", - }, - ], - binaries: ["/usr/local/bin/node", "/usr/bin/node", "/usr/local/bin/curl", "/usr/bin/curl"], - inference_capable: false, -}); - const { HOSTED_INFERENCE_ENDPOINT_URL, HOSTED_INFERENCE_MODEL, @@ -153,7 +128,6 @@ const { options?: { allowedSandboxes?: readonly string[]; bestEffort?: boolean; - gatewayName?: string; replaceExisting?: boolean; revalidateSandboxIdentity?(operation: string): void; requireExactBindings?: boolean; @@ -704,11 +678,8 @@ describe("onboard provider helpers", () => { ], (command) => { commands.push(command.join(" ")); - return command[1] === "profile" - ? { status: 0, stdout: BRAVE_PROFILE_EXPORT, stderr: "" } - : command.includes("get") - ? { status: 1, stdout: "", stderr: "" } - : { status: 0, stdout: "", stderr: "" }; + if (command.includes("get")) return { status: 1, stdout: "", stderr: "" }; + return { status: 0, stdout: "", stderr: "" }; }, ); @@ -951,9 +922,7 @@ describe("onboard provider helpers", () => { ], (command) => { commands.push(command.join(" ")); - return command[1] === "profile" - ? { status: 0, stdout: BRAVE_PROFILE_EXPORT, stderr: "" } - : { status: 0, stdout: "", stderr: "" }; + return { status: 0, stdout: "", stderr: "" }; }, ); @@ -961,7 +930,7 @@ describe("onboard provider helpers", () => { // still attached to a live sandbox, so reuse paths must use `update`. expect(providers).toEqual(["alpha-brave-search"]); expect(commands).toEqual([ - "provider profile export brave --output json", + expect.stringContaining("nemoclaw-blueprint/provider-profiles/brave.yaml"), "provider get alpha-brave-search", "provider update alpha-brave-search --credential BRAVE_API_KEY", ]); @@ -1120,11 +1089,7 @@ describe("onboard provider helpers", () => { } }); - it("reports a valid unscoped recovery command when bridge refresh throws (#9833)", () => { - const runResults = new Map([ - ["get", { status: 1, stdout: "", stderr: "not found" }], - ["delete", { status: 1, stdout: "", stderr: "gateway unavailable" }], - ]); + it("reports providers changed before bridge refresh throws (#9833)", () => { const configureRefreshes = vi .spyOn(messagingBridgeProvider, "configureMessagingBridgeRefreshes") .mockImplementation(() => { @@ -1134,14 +1099,12 @@ describe("onboard provider helpers", () => { expect(() => upsertMessagingProviders( [{ name: "alpha-bridge", envKey: "BRIDGE_TOKEN", token: "test-token" }], - (command) => runResults.get(command[1] ?? "") ?? { status: 0, stdout: "", stderr: "" }, + () => ({ status: 0, stdout: "", stderr: "" }), { bestEffort: true }, ), ).toThrow( expect.objectContaining({ - message: expect.stringMatching( - /sandbox identity changed.*alpha-bridge.*openshell provider delete "alpha-bridge"/isu, - ), + message: expect.stringMatching(/sandbox identity changed.*alpha-bridge/isu), mutatedProviderNames: ["alpha-bridge"], }), ); @@ -1172,53 +1135,53 @@ describe("onboard provider helpers", () => { } }); - it("preserves an exact existing bridge provider when refresh fails", () => { - const commands: string[] = []; + it("reuses the named bridge provider when onboarding retries after a mint failure", () => { const ensureProfiles = vi .spyOn(messagingBridgeProvider, "ensureMessagingBridgeProfiles") .mockImplementation(() => undefined); const configureRefreshes = vi .spyOn(messagingBridgeProvider, "configureMessagingBridgeRefreshes") - .mockReturnValue({ ok: false, reason: "refresh failed" }); - try { - expect(() => - upsertMessagingProviders( - [ - { - name: "alpha-googlechat-bridge", - envKey: "GOOGLE_CHAT_ACCESS_TOKEN", - token: messagingBridgeProvider.MESSAGING_BRIDGE_PENDING_VALUE, - providerType: "google-chat-bridge", - }, - ], - (command) => { - commands.push(command.join(" ")); - return { + .mockReturnValueOnce({ ok: false, reason: "mint timed out" }) + .mockReturnValueOnce({ ok: true }); + const commands: string[] = []; + let providerExists = false; + const run = (command: string[]) => { + commands.push(command.join(" ")); + if (command[1] === "get") { + return providerExists + ? { status: 0, - stdout: [ - "Name: alpha-googlechat-bridge", - "Type: google-chat-bridge", - "Credential keys: GOOGLE_CHAT_ACCESS_TOKEN", - "Config keys: ", - ].join("\n"), - stderr: "", - }; - }, - { bestEffort: true, gatewayName: "test-gateway" }, - ), - ).toThrow( - expect.objectContaining({ - message: expect.stringMatching(/gateway token minting/u), - mutatedProviderNames: ["alpha-googlechat-bridge"], - }), - ); - expect(commands).toEqual(["provider get alpha-googlechat-bridge"]); - expect(commands.some((command) => /provider (create|update|delete)/u.test(command))).toBe( - false, + stdout: + "Name: alpha-googlechat-bridge\nType: google-chat-bridge\nCredential keys: GOOGLE_CHAT_ACCESS_TOKEN\nConfig keys: \n", + } + : { status: 1, stderr: "provider not found" }; + } + if (command[1] === "create") providerExists = true; + return { status: 0, stdout: "", stderr: "" }; + }; + const tokenDefs = [ + { + name: "alpha-googlechat-bridge", + envKey: "GOOGLE_CHAT_ACCESS_TOKEN", + token: "openshell-managed-pending-mint", + providerType: "google-chat-bridge", + }, + ]; + + try { + expect(() => upsertMessagingProviders(tokenDefs, run, { bestEffort: true })).toThrow( + /gateway token minting/u, ); + expect(upsertMessagingProviders(tokenDefs, run, { bestEffort: true })).toEqual([ + "alpha-googlechat-bridge", + ]); + + expect(commands.filter((command) => command.includes("provider create"))).toHaveLength(1); + expect(ensureProfiles).toHaveBeenCalledTimes(2); + expect(configureRefreshes).toHaveBeenCalledTimes(2); } finally { - configureRefreshes.mockRestore(); ensureProfiles.mockRestore(); + configureRefreshes.mockRestore(); } }); @@ -1329,16 +1292,14 @@ describe("onboard provider helpers", () => { ], (command) => { commands.push(command.join(" ")); - return command[1] === "profile" - ? { status: 0, stdout: BRAVE_PROFILE_EXPORT, stderr: "" } - : { status: 0, stdout: "", stderr: "" }; + return { status: 0, stdout: "", stderr: "" }; }, { replaceExisting: true }, ); expect(providers).toEqual(["alpha-brave-search"]); expect(commands).toEqual([ - "provider profile export brave --output json", + expect.stringContaining("nemoclaw-blueprint/provider-profiles/brave.yaml"), "provider get alpha-brave-search", "provider delete alpha-brave-search", "provider create --name alpha-brave-search --type brave --credential BRAVE_API_KEY", diff --git a/test/channels/channels-add-bridge-lifecycle.test.ts b/test/channels/channels-add-bridge-lifecycle.test.ts index 4f3303d4ad7..18cb3b33ffb 100644 --- a/test/channels/channels-add-bridge-lifecycle.test.ts +++ b/test/channels/channels-add-bridge-lifecycle.test.ts @@ -4,13 +4,14 @@ // Bridge-provider lifecycle on the DIRECT `channels add` path (#6120): a // bridge-backed channel (googlechat) declares no manifest credentials, so the // add path must (1) create + refresh-configure the gateway bridge provider -// itself, (2) exit with an error when the pasted secret is missing, and (3) -// detach and delete the newly created provider when gateway registration fails. +// itself, (2) fail loudly when the pasted secret is missing, and (3) tear the +// just-created provider back down when gateway registration fails midway. import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from "vitest"; +import YAML from "yaml"; import { addSandboxChannel, removeSandboxChannel, @@ -44,55 +45,16 @@ const GOOGLECHAT_ENV = { GOOGLECHAT_AUDIENCE: "https://bot.example.com/googlechat", GOOGLECHAT_APP_PRINCIPAL: "123456789012345678901", }; -// Deliberately independent of the checked-in YAML read by production. If that -// contract changes without the simulated gateway export changing too, the real -// adapter comparison in this lifecycle test must fail. -const GOOGLECHAT_PROFILE_DOC: Record = { - id: "google-chat-bridge", - credentials: [ - { - name: "access_token", - env_vars: ["GOOGLE_CHAT_ACCESS_TOKEN"], - required: true, - auth_style: "bearer", - header_name: "Authorization", - query_param: "", - refresh: { - strategy: "google-service-account-jwt", - scopes: ["https://www.googleapis.com/auth/chat.bot"], - material: [ - { - name: "client_email", - description: "Service-account client email (JWT issuer)", - required: true, - }, - { - name: "private_key", - description: "Service-account RSA private key (PEM); signs the JWT assertion", - required: true, - secret: true, - }, - { - name: "scope", - description: "OAuth scope(s) to mint the token for", - }, - ], - }, - }, - ], - endpoints: [ - { - host: "chat.googleapis.com", - port: 443, - protocol: "rest", - access: "read-write", - enforcement: "enforce", - }, - ], - binaries: ["/usr/local/bin/node", "/usr/bin/node"], - inference_capable: false, -}; const LIVE_IDENTITY_FINGERPRINT = "a".repeat(64); +const GOOGLECHAT_PROFILE_DOC = YAML.parse( + fs.readFileSync( + path.join( + process.cwd(), + "src/lib/messaging/channels/googlechat/provider-profile/openclaw.yaml", + ), + "utf-8", + ), +) as Record; // 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 @@ -132,16 +94,6 @@ let registryEntry: SandboxEntry; let appliedPresets: string[]; let session: onboardSession.Session; let stdinIsTty: PropertyDescriptor | undefined; -let gatewayCallCount: number; -let bridgeRefreshWasSecure: boolean; -let bridgeProfileRegistered: boolean; -let bridgeProfileWasImported: boolean; -let detachedProviders: Set; -let deletedProviders: Set; -let registeredProviders: Set; -let bridgeRefreshError: string | null; -let bridgeRefreshStatusError: string | null; -let providerDeleteError: string | null; function printedText(): string { return [...logSpy.mock.calls, ...errorSpy.mock.calls] @@ -154,12 +106,8 @@ function withoutGateway(args: readonly string[]): string[] { return index < 0 ? [...args] : [...args.slice(0, index), ...args.slice(index + 2)]; } -function resetGatewayObservations(): void { - gatewayCallCount = 0; - bridgeRefreshWasSecure = false; - bridgeProfileWasImported = false; - detachedProviders.clear(); - deletedProviders.clear(); +function openshellCalls(): string[][] { + return runOpenshellSpy.mock.calls.map((call) => withoutGateway(call[0] as string[])); } beforeEach(() => { @@ -247,85 +195,21 @@ beforeEach(() => { return command[0] === "provider" && command[1] === "refresh" && command[2] === "status"; }; - gatewayCallCount = 0; - bridgeRefreshWasSecure = false; - bridgeProfileRegistered = false; - bridgeProfileWasImported = false; - detachedProviders = new Set(); - deletedProviders = new Set(); - registeredProviders = new Set(); - bridgeRefreshError = null; - bridgeRefreshStatusError = null; - providerDeleteError = null; - runOpenshellSpy = vi.spyOn(runtime, "runOpenshell").mockImplementation((args, options) => { - gatewayCallCount += 1; + runOpenshellSpy = vi.spyOn(runtime, "runOpenshell").mockImplementation((args) => { const command = withoutGateway(args); - const providerName = command[0] === "provider" && command[1] === "get" ? command[2] : null; - const providerMissing = Boolean(providerName && !registeredProviders.has(providerName)); - const exportingProfile = - command[0] === "provider" && command[1] === "profile" && command.includes("export"); - const importingProfile = - command[0] === "provider" && command[1] === "profile" && command.includes("import"); - const profileMissing = exportingProfile && !bridgeProfileRegistered; - bridgeProfileRegistered ||= importingProfile; - bridgeProfileWasImported ||= importingProfile; - const detachedProvider = - command[0] === "sandbox" && command[1] === "provider" && command[2] === "detach" - ? command[4] - : null; - const deletedProvider = - command[0] === "provider" && command[1] === "delete" ? command[2] : null; - const createdProvider = - command[0] === "provider" && command[1] === "create" - ? command[command.indexOf("--name") + 1] - : null; - const configuringRefresh = - command[0] === "provider" && command[1] === "refresh" && command[2] === "configure"; - const readingRefreshStatus = isRefreshStatus(args); - const refreshFailure = configuringRefresh ? bridgeRefreshError : null; - const refreshStatusFailure = readingRefreshStatus ? bridgeRefreshStatusError : null; - const deleteFailure = deletedProvider ? providerDeleteError : null; - const commandFailure = refreshFailure ?? deleteFailure ?? ""; - detachedProvider ? detachedProviders.add(detachedProvider) : undefined; - deletedProvider ? deletedProviders.add(deletedProvider) : undefined; - deletedProvider && !deleteFailure ? registeredProviders.delete(deletedProvider) : undefined; - createdProvider ? registeredProviders.add(createdProvider) : undefined; - const runEnv = options?.env as Record | undefined; - bridgeRefreshWasSecure = configuringRefresh - ? command.includes("--secret-material-env") && - command.includes("private_key=MESSAGING_BRIDGE_SECRET_0") && - !command.join(" ").includes("fake-test-private-key-material") && - runEnv?.MESSAGING_BRIDGE_SECRET_0 === "fake-test-private-key-material" - : bridgeRefreshWasSecure; - const invalidRefresh = configuringRefresh && !bridgeRefreshWasSecure; - refreshStatusFailure - ? (() => { - throw new Error(refreshStatusFailure); - })() - : undefined; + const providerMissing = command[0] === "provider" && command[1] === "get"; + const profileExport = + command[0] === "provider" && command[1] === "profile" && command[2] === "export"; return { pid: 0, output: [null, "", ""], - stdout: readingRefreshStatus + stdout: isRefreshStatus(args) ? refreshStatusTable(command) - : exportingProfile && !profileMissing + : profileExport ? JSON.stringify(GOOGLECHAT_PROFILE_DOC) - : providerName && !providerMissing - ? `Name: ${providerName}\nType: google-chat-bridge\nCredential keys: GOOGLE_CHAT_ACCESS_TOKEN\nConfig keys: \n` : "", - stderr: invalidRefresh - ? "invalid secret handoff" - : commandFailure - ? commandFailure - : profileMissing - ? "provider profile 'google-chat-bridge' not found" - : providerMissing - ? `provider '${args[args.length - 1]}' not found` - : "", - status: - invalidRefresh || refreshFailure || deleteFailure || profileMissing || providerMissing - ? 1 - : 0, + stderr: providerMissing ? `provider '${args[args.length - 1]}' not found` : "", + status: providerMissing ? 1 : 0, signal: null, }; }); @@ -377,8 +261,20 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { "nemoclaw", { bestEffort: true, requireExactBindings: true }, ); - expect(bridgeProfileWasImported).toBe(true); - expect(bridgeRefreshWasSecure).toBe(true); + const refreshCall = runOpenshellSpy.mock.calls.find( + (call) => + withoutGateway(call[0] as string[]) + .slice(0, 3) + .join(" ") === "provider refresh configure", + ); + expect(refreshCall).toBeDefined(); + const refreshArgs = refreshCall?.[0] as string[]; + expect(refreshArgs).toContain("--secret-material-env"); + expect(refreshArgs).toContain("private_key=MESSAGING_BRIDGE_SECRET_0"); + expect(refreshArgs.join(" ")).not.toContain("fake-test-private-key-material"); + expect(refreshCall?.[1]).toMatchObject({ + env: { MESSAGING_BRIDGE_SECRET_0: "fake-test-private-key-material" }, + }); expect(JSON.stringify({ registryEntry, session })).not.toContain( "fake-test-private-key-material", ); @@ -397,7 +293,7 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { expect(printedText()).toContain("Change queued."); }); - it("exits with an error at add time when the bridge secret is not resolvable", async () => { + it("fails loudly at add time when the bridge secret is not resolvable", async () => { delete process.env.GOOGLECHAT_SERVICE_ACCOUNT; await expect(addSandboxChannel("test-sb", { channel: "googlechat" })).rejects.toMatchObject({ @@ -406,11 +302,10 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { expect(exitSpy).toHaveBeenCalledWith(1); expect(providerSpy).not.toHaveBeenCalled(); - expect(printedText()).toContain("Missing required inputs for this channel."); - expect(printedText()).not.toContain("GOOGLECHAT_SERVICE_ACCOUNT"); + expect(printedText()).toContain("GOOGLECHAT_SERVICE_ACCOUNT"); }); - it("detaches and deletes the newly created bridge provider when registration fails", async () => { + it("tears the just-created bridge provider back down when gateway registration fails", async () => { providerSpy.mockImplementation(() => { throw new Error("simulated gateway failure"); }); @@ -419,49 +314,13 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { code: 1, }); - expect(printedText()).toContain("Failed to register channel providers with the gateway."); - expect(detachedProviders.has("test-sb-googlechat-bridge")).toBe(true); - expect(deletedProviders.has("test-sb-googlechat-bridge")).toBe(true); - }); - - it("reports scoped recovery when refresh cleanup leaves a bridge provider", async () => { - bridgeRefreshError = "refresh unavailable"; - providerDeleteError = "gateway unavailable"; - - await expect(addSandboxChannel("test-sb", { channel: "googlechat" })).rejects.toMatchObject({ - code: 1, - }); - - const diagnostics = printedText(); - expect(diagnostics).toContain("test-sb-googlechat-bridge"); - expect(diagnostics).toContain("gateway unavailable"); - expect(diagnostics).toContain( - 'openshell provider delete -g "nemoclaw" "test-sb-googlechat-bridge"', - ); - }); - - it("reports an uncertain existing provider when refresh status inspection throws", async () => { - bridgeProfileRegistered = true; - registeredProviders.add("test-sb-googlechat-bridge"); - bridgeRefreshStatusError = `status inspection failed: ${SA_JSON}`; - - await expect(addSandboxChannel("test-sb", { channel: "googlechat" })).rejects.toMatchObject({ - code: 1, - }); - - const diagnostics = printedText(); - expect(diagnostics).toContain("test-sb-googlechat-bridge"); - expect(diagnostics).toContain("status inspection failed"); - expect(diagnostics).toContain("inspect the named provider"); - expect(diagnostics).toContain("correct the gateway failure"); - expect(diagnostics).not.toContain(SA_JSON); - expect(diagnostics).not.toContain("fake-test-private-key-material"); - expect(diagnostics).not.toContain("fake"); - expect(deletedProviders.has("test-sb-googlechat-bridge")).toBe(false); - expect(registry.getConfiguredMessagingChannelsFromEntry(registryEntry)).not.toContain( - "googlechat", + expect(printedText()).toContain("Failed to register 'googlechat' providers"); + expect(openshellCalls()).toEqual( + expect.arrayContaining([ + ["sandbox", "provider", "detach", "test-sb", "test-sb-googlechat-bridge"], + ["provider", "delete", "test-sb-googlechat-bridge"], + ]), ); - expect(session.messagingPlan).toBeUndefined(); }); it("removes the bridge provider, policy, and durable plan through the channel action", async () => { @@ -470,11 +329,14 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { expect(appliedPresets).toContain("googlechat"); runOpenshellSpy.mockClear(); - resetGatewayObservations(); await removeSandboxChannel("test-sb", { channel: "googlechat" }); - expect(detachedProviders.has("test-sb-googlechat-bridge")).toBe(true); - expect(deletedProviders.has("test-sb-googlechat-bridge")).toBe(true); + expect(openshellCalls()).toEqual( + expect.arrayContaining([ + ["sandbox", "provider", "detach", "test-sb", "test-sb-googlechat-bridge"], + ["provider", "delete", "test-sb-googlechat-bridge"], + ]), + ); expect(registry.getConfiguredMessagingChannelsFromEntry(registryEntry)).not.toContain( "googlechat", ); @@ -489,7 +351,6 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { providerSpy.mockClear(); runOpenshellSpy.mockClear(); - resetGatewayObservations(); vi.mocked(policies.removePreset).mockClear(); vi.mocked(registry.updateSandbox).mockClear(); vi.mocked(policyChannelDependencies.rebuildSandbox).mockClear(); @@ -508,7 +369,7 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { expect(registry.getConfiguredMessagingChannelsFromEntry(registryEntry)).toContain("googlechat"); expect(appliedPresets).toContain("googlechat"); expect(providerSpy).not.toHaveBeenCalled(); - expect(gatewayCallCount).toBe(0); + expect(openshellCalls()).toEqual([]); expect(policies.removePreset).not.toHaveBeenCalled(); expect(registry.updateSandbox).not.toHaveBeenCalled(); expect(policyChannelDependencies.rebuildSandbox).not.toHaveBeenCalled(); @@ -518,7 +379,6 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { await addSandboxChannel("test-sb", { channel: "googlechat" }); providerSpy.mockClear(); runOpenshellSpy.mockClear(); - resetGatewayObservations(); stopGooglechatWebhookTunnelSpy.mockClear(); vi.mocked(policies.applyPreset).mockClear(); @@ -533,7 +393,7 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { ]), ); expect(providerSpy).not.toHaveBeenCalled(); - expect(gatewayCallCount).toBe(0); + expect(openshellCalls()).toEqual([]); expect(stopGooglechatWebhookTunnelSpy).not.toHaveBeenCalled(); await startSandboxChannel("test-sb", { channel: "googlechat" }); @@ -550,7 +410,7 @@ describe("channels add owns the bridge-provider lifecycle (#6120)", () => { expect(policies.applyPreset).not.toHaveBeenCalled(); expect(appliedPresets).toContain("googlechat"); expect(providerSpy).not.toHaveBeenCalled(); - expect(gatewayCallCount).toBe(0); + expect(openshellCalls()).toEqual([]); expect(stopGooglechatWebhookTunnelSpy).not.toHaveBeenCalled(); }); }); diff --git a/test/e2e/live/channels-stop-start-helpers.ts b/test/e2e/live/channels-stop-start-helpers.ts index e2d5a94b890..31932fef657 100644 --- a/test/e2e/live/channels-stop-start-helpers.ts +++ b/test/e2e/live/channels-stop-start-helpers.ts @@ -9,6 +9,7 @@ import type { AddSandboxChannelDependencies } from "../../../src/lib/actions/san import * as policyChannelDependenciesModule from "../../../src/lib/actions/sandbox/policy-channel-dependencies.ts"; import * as policyChannelModule from "../../../src/lib/actions/sandbox/policy-channel.ts"; import * as openshellRuntimeModule from "../../../src/lib/adapters/openshell/runtime.ts"; +import * as credentialProviderRegistrationModule from "../../../src/lib/onboard/credential-provider-registration.ts"; import * as messagingBridgeProviderModule from "../../../src/lib/onboard/messaging-bridge-provider.ts"; import * as legacyProvidersModule from "../../../src/lib/onboard/providers.ts"; import { clearStoppedSandboxStateRoots } from "../../../src/lib/sandbox/privileged-exec.ts"; @@ -56,6 +57,8 @@ type PolicyChannelModule = typeof import("../../../src/lib/actions/sandbox/polic type PolicyChannelDependenciesModule = typeof import("../../../src/lib/actions/sandbox/policy-channel-dependencies.ts"); type OpenshellRuntimeModule = typeof import("../../../src/lib/adapters/openshell/runtime.ts"); +type CredentialProviderRegistrationModule = + typeof import("../../../src/lib/onboard/credential-provider-registration.ts"); type MessagingBridgeProviderModule = typeof import("../../../src/lib/onboard/messaging-bridge-provider.ts"); type StatePathsModule = typeof import("../../../src/lib/state/paths.ts"); @@ -91,6 +94,13 @@ const messagingBridgeProvider = ( : messagingBridgeProviderModule ) as MessagingBridgeProviderModule; const { ensureMessagingBridgeProfiles } = messagingBridgeProvider; +const credentialProviderRegistration = ( + "default" in credentialProviderRegistrationModule + ? credentialProviderRegistrationModule.default + : credentialProviderRegistrationModule +) as CredentialProviderRegistrationModule; +const credentialProviderRegistrationDependencies = + credentialProviderRegistration.credentialProviderRegistrationDependencies as ProviderDependencies; const legacyProviderDependencies = ( "default" in legacyProvidersModule ? legacyProvidersModule.default : legacyProvidersModule ) as ProviderDependencies; @@ -223,9 +233,12 @@ export function installGooglechatCredentialFixture( const registered = new Set([...delegatedProviderNames, expectedName]); return tokenDefs.map(({ name }) => name).filter((name) => registered.has(name)); }; - const providerDependencies = dependencies.providerDependencies ?? legacyProviderDependencies; + const providerDependencies = + dependencies.providerDependencies ?? credentialProviderRegistrationDependencies; + const injectedProviderDependencies = dependencies.providerDependencies !== undefined; const effectiveLegacyProviderDependencies = - dependencies.legacyProviderDependencies ?? providerDependencies; + dependencies.legacyProviderDependencies ?? + (injectedProviderDependencies ? providerDependencies : legacyProviderDependencies); const root = dependencies.root ?? ROOT; const run = dependencies.run ?? runOpenshell; const originalRegistrationUpsert = providerDependencies.upsertMessagingProviders; diff --git a/test/e2e/support/channels-stop-start-googlechat.test.ts b/test/e2e/support/channels-stop-start-googlechat.test.ts index 872be81a130..25822799a2b 100644 --- a/test/e2e/support/channels-stop-start-googlechat.test.ts +++ b/test/e2e/support/channels-stop-start-googlechat.test.ts @@ -3,11 +3,131 @@ import { describe, expect, it, vi } from "vitest"; +import { credentialProviderRegistrationDependencies } from "../../../src/lib/onboard/credential-provider-registration.ts"; +import * as legacyProvidersModule from "../../../src/lib/onboard/providers.ts"; import { addAndRebuildGooglechatForChannelsStopStartLiveE2e, + GOOGLECHAT_E2E_ACCESS_TOKEN, + installGooglechatCredentialFixture, rebuildGooglechatForChannelsStopStartLiveE2e, } from "../live/channels-stop-start-helpers.ts"; + +type FixtureRunner = typeof import("../../../src/lib/adapters/openshell/runtime.ts").runOpenshell; +type FixtureProviderDependencies = { + upsertMessagingProviders( + tokenDefs: Parameters< + (typeof import("../../../src/lib/actions/sandbox/policy-channel-dependencies.ts"))["policyChannelDependencies"]["upsertMessagingProviders"] + >[0], + run: FixtureRunner, + options?: { + readonly replaceExisting?: boolean; + readonly revalidateSandboxIdentity?: (operation: string) => void; + }, + ): string[]; +}; + +type FixtureChannelDependencies = Pick< + (typeof import("../../../src/lib/actions/sandbox/policy-channel-dependencies.ts"))["policyChannelDependencies"], + "runGatewayOpenshell" | "upsertMessagingProviders" +>; + +const legacyProviderDependencies = ( + "default" in legacyProvidersModule ? legacyProvidersModule.default : legacyProvidersModule +) as FixtureProviderDependencies; + describe("channels stop/start Google Chat live composition", () => { + it("intercepts the live policy-channel boundary before gateway refresh minting", () => { + const sandboxName = "e2e-oc-ch-cycle"; + const expectedName = `${sandboxName}-googlechat-bridge`; + const calls: string[][] = []; + const originalUpsert = vi.fn(() => []); + const channelDependencies: FixtureChannelDependencies = { + upsertMessagingProviders: originalUpsert, + runGatewayOpenshell: vi.fn((_gatewayName, args) => { + calls.push(args); + return { status: args[1] === "get" ? 1 : 0 } as never; + }), + }; + const restore = installGooglechatCredentialFixture(sandboxName, "openclaw", { + channelDependencies, + ensureProfiles: vi.fn(), + providerDependencies: { upsertMessagingProviders: vi.fn(() => []) }, + root: "/repo", + }); + + expect( + restore.upsertMessagingProviders( + [ + { + name: expectedName, + envKey: "GOOGLE_CHAT_ACCESS_TOKEN", + token: null, + providerType: "google-chat-bridge", + }, + ], + "nemoclaw", + { bestEffort: true, requireExactBindings: true }, + ), + ).toEqual([expectedName]); + expect(originalUpsert).not.toHaveBeenCalled(); + expect(channelDependencies.upsertMessagingProviders).toBe(originalUpsert); + expect(calls).toContainEqual([ + "provider", + "create", + "--name", + expectedName, + "--type", + "google-chat-bridge", + "--credential", + "GOOGLE_CHAT_ACCESS_TOKEN", + ]); + + restore(); + expect(channelDependencies.upsertMessagingProviders).toBe(originalUpsert); + }); + + it("routes rebuild registration through the scoped provider seam and restores it", () => { + const sandboxName = "e2e-oc-ch-cycle"; + const expectedName = `${sandboxName}-googlechat-bridge`; + const runMock = vi.fn((args: string[]) => ({ + status: args[1] === "get" ? 1 : 0, + stdout: "", + stderr: "", + })); + const run = runMock as unknown as FixtureRunner; + const originalUpsert = credentialProviderRegistrationDependencies.upsertMessagingProviders; + const originalLegacyUpsert = legacyProviderDependencies.upsertMessagingProviders; + const restore = installGooglechatCredentialFixture(sandboxName, "openclaw", { + ensureProfiles: vi.fn(), + root: "/repo", + run, + }); + try { + expect( + credentialProviderRegistrationDependencies.upsertMessagingProviders( + [ + { + name: expectedName, + envKey: "GOOGLE_CHAT_ACCESS_TOKEN", + token: null, + providerType: "google-chat-bridge", + }, + ], + run, + {}, + ), + ).toEqual([expectedName]); + expect(runMock.mock.calls.some(([args]) => args[1] === "create")).toBe(true); + expect(runMock.mock.calls.some(([args]) => args.includes("refresh"))).toBe(false); + } finally { + restore(); + } + expect(credentialProviderRegistrationDependencies.upsertMessagingProviders).toBe( + originalUpsert, + ); + expect(legacyProviderDependencies.upsertMessagingProviders).toBe(originalLegacyUpsert); + }); + it("grants a process-local audience capability to the exact live sandbox", async () => { const addSandboxChannel = vi.fn(async () => {}); const rebuildSandbox = vi.fn(async () => {}); @@ -195,4 +315,191 @@ describe("channels stop/start Google Chat live composition", () => { expect(restore).toHaveBeenCalledOnce(); }); + it.each([ + ["openclaw", "e2e-oc-ch-cycle", "google-chat-bridge"], + ["hermes", "e2e-hm-ch-cycle", "google-chat-hermes-bridge"], + ] as const)( + "creates the real %s provider profile without putting the fixture value in argv", + (agent, sandboxName, providerType) => { + const delegatedName = `${sandboxName}-slack-bridge`; + const delegatedTokenDef = { + name: delegatedName, + envKey: "SLACK_BOT_TOKEN", + token: "e2e-fake-slack-token", + providerType: "nemoclaw-mcp-v1", + }; + const originalUpsert = vi.fn(() => [delegatedName]); + const providerDependencies: FixtureProviderDependencies = { + upsertMessagingProviders: originalUpsert, + }; + const ensureProfiles = vi.fn(); + const runMock = vi.fn((args: string[], _options?: { env?: NodeJS.ProcessEnv }) => ({ + status: args[1] === "get" ? 1 : 0, + })); + const run = runMock as unknown as FixtureRunner; + const revalidateSandboxIdentity = vi.fn(); + + const restore = installGooglechatCredentialFixture(sandboxName, agent, { + ensureProfiles, + providerDependencies, + root: "/repo", + run, + }); + const providerNames = providerDependencies.upsertMessagingProviders( + [ + delegatedTokenDef, + { + name: `${sandboxName}-googlechat-bridge`, + envKey: "GOOGLE_CHAT_ACCESS_TOKEN", + token: null, + providerType, + }, + ], + run, + { revalidateSandboxIdentity }, + ); + + expect(providerNames).toEqual([delegatedName, `${sandboxName}-googlechat-bridge`]); + expect(originalUpsert).toHaveBeenCalledWith([delegatedTokenDef], run, { + revalidateSandboxIdentity, + }); + expect(ensureProfiles).toHaveBeenCalledOnce(); + const profileDependencies = ensureProfiles.mock.calls[0]?.[1] as { + redact: (value: string) => string; + root: string; + runOpenshell: FixtureRunner; + }; + expect(profileDependencies.root).toBe("/repo"); + expect(profileDependencies.runOpenshell).not.toBe(run); + expect(profileDependencies.redact(GOOGLECHAT_E2E_ACCESS_TOKEN)).toBe("[redacted]"); + expect(revalidateSandboxIdentity).toHaveBeenCalledTimes(2); + + const createCall = runMock.mock.calls.find(([args]) => args[1] === "create"); + expect(createCall?.[0]).toEqual([ + "provider", + "create", + "--name", + `${sandboxName}-googlechat-bridge`, + "--type", + providerType, + "--credential", + "GOOGLE_CHAT_ACCESS_TOKEN", + ]); + expect(createCall?.[0]).not.toContain(GOOGLECHAT_E2E_ACCESS_TOKEN); + expect(createCall?.[1]?.env).toMatchObject({ + GOOGLE_CHAT_ACCESS_TOKEN: GOOGLECHAT_E2E_ACCESS_TOKEN, + }); + + restore(); + expect(providerDependencies.upsertMessagingProviders).toBe(originalUpsert); + }, + ); + + it("intercepts both registration and legacy provider boundaries during rebuild", () => { + const sandboxName = "e2e-oc-ch-cycle"; + const expectedName = `${sandboxName}-googlechat-bridge`; + const delegatedName = `${sandboxName}-slack-bridge`; + const registrationOriginal = vi.fn(() => []); + const legacyOriginal = vi.fn(() => [delegatedName]); + const providerDependencies: FixtureProviderDependencies = { + upsertMessagingProviders: registrationOriginal, + }; + const legacyProviderDependencies: FixtureProviderDependencies = { + upsertMessagingProviders: legacyOriginal, + }; + const run = vi.fn((args: string[]) => ({ + status: args[1] === "get" ? 1 : 0, + })) as unknown as FixtureRunner; + const restore = installGooglechatCredentialFixture(sandboxName, "openclaw", { + ensureProfiles: vi.fn(), + providerDependencies, + legacyProviderDependencies, + root: "/repo", + run, + }); + + expect(providerDependencies.upsertMessagingProviders).toBe( + legacyProviderDependencies.upsertMessagingProviders, + ); + expect( + providerDependencies.upsertMessagingProviders( + [ + { + name: delegatedName, + envKey: "SLACK_BOT_TOKEN", + token: "e2e-fake-slack-token", + providerType: "nemoclaw-mcp-v1", + }, + { + name: expectedName, + envKey: "GOOGLE_CHAT_ACCESS_TOKEN", + token: null, + providerType: "google-chat-bridge", + }, + ], + run, + ), + ).toEqual([delegatedName, expectedName]); + expect(registrationOriginal).not.toHaveBeenCalled(); + expect(legacyOriginal).toHaveBeenCalledOnce(); + + restore(); + expect(providerDependencies.upsertMessagingProviders).toBe(registrationOriginal); + expect(legacyProviderDependencies.upsertMessagingProviders).toBe(legacyOriginal); + }); + + it.each([ + [{}, "update"], + [{ replaceExisting: true }, "create"], + ] as const)( + "reconciles an existing fixture provider with options %o", + (options, expectedMutation) => { + const providerDependencies: FixtureProviderDependencies = { + upsertMessagingProviders: vi.fn(() => []), + }; + const calls: Array<{ args: string[]; env?: Record }> = []; + const run = ((args: string[], runOptions?: { env?: Record }) => { + calls.push({ args, env: runOptions?.env }); + return { status: 0 }; + }) as unknown as FixtureRunner; + const restore = installGooglechatCredentialFixture("e2e-oc-ch-cycle", "openclaw", { + ensureProfiles: vi.fn(), + providerDependencies, + root: "/repo", + run, + }); + + providerDependencies.upsertMessagingProviders( + [ + { + name: "e2e-oc-ch-cycle-googlechat-bridge", + envKey: "GOOGLE_CHAT_ACCESS_TOKEN", + token: null, + providerType: "google-chat-bridge", + }, + ], + run, + options, + ); + + const mutation = calls.find(({ args }) => args[1] === expectedMutation); + expect(mutation?.args).toEqual( + expect.arrayContaining([ + "e2e-oc-ch-cycle-googlechat-bridge", + "--credential", + "GOOGLE_CHAT_ACCESS_TOKEN", + ]), + ); + if (expectedMutation === "create") { + expect(mutation?.args).toEqual( + expect.arrayContaining(["--type", "google-chat-bridge"]), + ); + } + expect(mutation?.env).toEqual({ + GOOGLE_CHAT_ACCESS_TOKEN: GOOGLECHAT_E2E_ACCESS_TOKEN, + }); + expect(calls.flatMap(({ args }) => args)).not.toContain(GOOGLECHAT_E2E_ACCESS_TOKEN); + restore(); + }, + ); }); diff --git a/test/helpers/onboard-final-flow-phases.ts b/test/helpers/onboard-final-flow-phases.ts index eb96b9aaf1c..6f4ddee4aae 100644 --- a/test/helpers/onboard-final-flow-phases.ts +++ b/test/helpers/onboard-final-flow-phases.ts @@ -223,6 +223,7 @@ export function createPhases( mergePolicyMessagingChannels: recorders.mergePolicyMessagingChannels ?? ((selected) => selected), detectUnconfiguredMessagingChannels: () => [], + providerMatchesGatewayCredential: () => false, verifyCompatibleEndpointSandboxSmoke: vi.fn(), preparePolicyPresetResumeSelection: () => ({ policyPresets: ["balanced"],