diff --git a/docs/reference/commands-nemohermes.mdx b/docs/reference/commands-nemohermes.mdx index 16b3c804c5..3875d1140b 100644 --- a/docs/reference/commands-nemohermes.mdx +++ b/docs/reference/commands-nemohermes.mdx @@ -756,6 +756,7 @@ nemohermes my-assistant channels add telegram | Flag | Description | |------|-------------| | `--dry-run` | Validate the channel name and matching policy preset without prompting for credentials, contacting the gateway, or rebuilding | +| `--force` | Add the channel even when another sandbox already uses the same messaging credential, bypassing the cross-sandbox conflict warning (otherwise `channels add` warns and, when non-interactive, aborts) | Slack requires both `SLACK_BOT_TOKEN` (bot user OAuth) and `SLACK_APP_TOKEN` (app-level Socket Mode token); the command prompts for each in turn. Optional Slack allowlists come from `SLACK_ALLOWED_USERS` and `SLACK_ALLOWED_CHANNELS` at rebuild time. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index d6cf836e5d..c2a8df4ba3 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -907,6 +907,7 @@ $$nemoclaw my-assistant channels add telegram | Flag | Description | |------|-------------| | `--dry-run` | Validate the channel name and matching policy preset without prompting for credentials, contacting the gateway, or rebuilding | +| `--force` | Add the channel even when another sandbox already uses the same messaging credential, bypassing the cross-sandbox conflict warning (otherwise `channels add` warns and, when non-interactive, aborts) | Slack requires both `SLACK_BOT_TOKEN` (bot user OAuth) and `SLACK_APP_TOKEN` (app-level Socket Mode token); the command prompts for each in turn. Optional Slack allowlists come from `SLACK_ALLOWED_USERS` and `SLACK_ALLOWED_CHANNELS` at rebuild time. diff --git a/src/commands/sandbox/channels/add.ts b/src/commands/sandbox/channels/add.ts index 6b1c05292e..b0be6622c9 100644 --- a/src/commands/sandbox/channels/add.ts +++ b/src/commands/sandbox/channels/add.ts @@ -7,7 +7,7 @@ import { NemoClawCommand } from "../../../lib/cli/nemoclaw-oclif-command"; import { channelMutationOptions, channelMutationArgs, - channelMutationFlags, + channelAddFlags, } from "../../../lib/sandbox/channels-command-support"; export default class ChannelsAddCommand extends NemoClawCommand { @@ -15,10 +15,10 @@ export default class ChannelsAddCommand extends NemoClawCommand { static strict = true; static summary = "Save messaging channel credentials and rebuild"; static description = "Store credentials for a messaging channel and queue a sandbox rebuild."; - static usage = [" [--dry-run]"]; + static usage = [" [--dry-run] [--force]"]; static examples = ["<%= config.bin %> sandbox channels add alpha telegram"]; static args = channelMutationArgs; - static flags = channelMutationFlags; + static flags = channelAddFlags; public async run(): Promise { const { args, flags } = await this.parse(ChannelsAddCommand); diff --git a/src/commands/sandbox/channels/mutate.test.ts b/src/commands/sandbox/channels/mutate.test.ts index f07cb00151..b232512467 100644 --- a/src/commands/sandbox/channels/mutate.test.ts +++ b/src/commands/sandbox/channels/mutate.test.ts @@ -30,6 +30,40 @@ describe("channels mutation oclif commands", () => { expect(mocks.addSandboxChannel).toHaveBeenCalledWith("alpha", { channel: "telegram", dryRun: true, + force: false, + }); + }); + + // Scenario 12 (#4305): --force is threaded through to addSandboxChannel. + it("threads --force through add to typed action options", async () => { + await ChannelsAddCommand.run(["alpha", "telegram", "--force"], rootDir); + + expect(mocks.addSandboxChannel).toHaveBeenCalledWith("alpha", { + channel: "telegram", + dryRun: false, + force: true, + }); + }); + + // Scenario 12 (#4305): omitting --force yields force:false (no implicit override). + it("defaults force to false when --force is omitted on add", async () => { + await ChannelsAddCommand.run(["alpha", "telegram"], rootDir); + + expect(mocks.addSandboxChannel).toHaveBeenCalledWith("alpha", { + channel: "telegram", + dryRun: false, + force: false, + }); + }); + + // Scenario 12 (#4305): --force combines with --dry-run independently. + it("threads both --force and --dry-run on add", async () => { + await ChannelsAddCommand.run(["alpha", "telegram", "--force", "--dry-run"], rootDir); + + expect(mocks.addSandboxChannel).toHaveBeenCalledWith("alpha", { + channel: "telegram", + dryRun: true, + force: true, }); }); @@ -41,17 +75,31 @@ describe("channels mutation oclif commands", () => { expect(mocks.removeSandboxChannel).toHaveBeenCalledWith("alpha", { channel: "telegram", dryRun: false, + force: false, }); expect(mocks.startSandboxChannel).toHaveBeenCalledWith("alpha", { channel: "telegram", dryRun: true, + force: false, }); expect(mocks.stopSandboxChannel).toHaveBeenCalledWith("alpha", { channel: "slack", dryRun: false, + force: false, }); }); + // Scenario 12 (#4305): --force is add-only. Only `channels add` can create a + // cross-sandbox credential overlap, so only it exposes the override; surfacing + // a no-op --force on remove/start/stop would mislead users and break the + // CLI/docs flag-parity check. + it("exposes --force only on add, not on remove/start/stop", () => { + expect(ChannelsAddCommand.flags).toHaveProperty("force"); + expect(ChannelsRemoveCommand.flags).not.toHaveProperty("force"); + expect(ChannelsStartCommand.flags).not.toHaveProperty("force"); + expect(ChannelsStopCommand.flags).not.toHaveProperty("force"); + }); + it("requires a channel before dispatch", async () => { await expect(ChannelsAddCommand.run(["alpha"], rootDir)).rejects.toThrow(/channel/i); diff --git a/src/lib/actions/sandbox/policy-channel-conflict.test.ts b/src/lib/actions/sandbox/policy-channel-conflict.test.ts new file mode 100644 index 0000000000..6858fe18a4 --- /dev/null +++ b/src/lib/actions/sandbox/policy-channel-conflict.test.ts @@ -0,0 +1,567 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Cross-sandbox messaging-credential conflict detection on `channels add` +// (issue #4305). These tests drive the public `addSandboxChannel` action and +// assert only on the mocked module boundaries — never on the private helper +// names — so they survive a refactor of the internal conflict-check plumbing. +// +// Why dist + vi.spyOn (the rebuild-shields-finally.test.ts pattern): the source +// policy-channel.ts loads several deps via runtime CommonJS `require()` +// (../../onboard, ../../onboard/providers, ./rebuild, ../../runner, ...). In +// this repo's vitest setup, `vi.mock` only intercepts ESM `import`, not plain +// `require()`, and those modules do extensionless sibling requires the TS +// transform cannot resolve. So we require the COMPILED module + its real +// compiled dependency modules from dist/ (one shared require cache) and +// `vi.spyOn` the dependency exports. Run `npm run build:cli` first. +// +// isNonInteractive is destructured at module load (`const { isNonInteractive } +// = require("../../onboard")`), so it cannot be spied after load; it reads +// process.env.NEMOCLAW_NON_INTERACTIVE === "1" at call time, which we drive +// directly. The real messaging-conflict, sandbox/channels, and credential-hash +// modules run unmocked so the genuine hash + conflict logic is exercised. + +import { createRequire } from "node:module"; + +import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from "vitest"; + +const requireDist = createRequire(import.meta.url); +const D = (p: string) => requireDist(`../../../../dist/lib/${p}`); + +type SandboxEntry = import("../../state/registry").SandboxEntry; + +// Real compiled dependency modules (shared require cache with the SUT). +const store = D("credentials/store.js"); +const registry = D("state/registry.js"); +const providers = D("onboard/providers.js"); +const runtime = D("adapters/openshell/runtime.js"); +const gatewayRuntime = D("gateway-runtime-action.js"); +const defs = D("agent/defs.js"); +const rebuild = D("actions/sandbox/rebuild.js"); +const processRecovery = D("actions/sandbox/process-recovery.js"); +const onboardSession = D("state/onboard-session.js"); +const slackValidation = D("actions/sandbox/slack-channel-validation.js"); +const policy = D("policy/index.js"); +const { hashCredential } = D("security/credential-hash.js") as { + hashCredential: (v: string) => string | null; +}; +const { addSandboxChannel } = D("actions/sandbox/policy-channel.js") as { + addSandboxChannel: ( + name: string, + options?: { channel?: string; dryRun?: boolean; force?: boolean }, + ) => Promise; +}; + +const TELEGRAM_TOKEN = "123456:AAH-secret-bot-token-value"; +const TELEGRAM_HASH = hashCredential(TELEGRAM_TOKEN) as string; + +let spies: MockInstance[]; +let logSpy: MockInstance; +let errSpy: MockInstance; +let exitMock: MockInstance; +let promptMock: MockInstance; +let getCredentialMock: MockInstance; +let updateSandboxMock: MockInstance; +let upsertMock: MockInstance; +let runOpenshellMock: MockInstance; +let applyPresetMock: MockInstance; +let getSandboxMock: MockInstance; +let listSandboxesMock: MockInstance; + +function arrangeRegistry(opts: { current: SandboxEntry; others?: SandboxEntry[] }): void { + const all = [opts.current, ...(opts.others ?? [])]; + listSandboxesMock.mockReturnValue({ sandboxes: all, defaultSandbox: opts.current.name }); + getSandboxMock.mockImplementation((name: string) => all.find((s) => s.name === name) ?? null); +} + +function loggedText(): string { + const lines: string[] = []; + for (const call of (logSpy.mock.calls as unknown[][]) ?? []) lines.push(call.map(String).join(" ")); + for (const call of (errSpy.mock.calls as unknown[][]) ?? []) lines.push(call.map(String).join(" ")); + return lines.join("\n"); +} + +// True iff the conflict-resolution "Continue anyway?" prompt was shown. +// (The unrelated "Rebuild now?" prompt fires after a successful add and must +// not be conflated with the conflict prompt.) +function conflictPromptShown(): boolean { + return (promptMock.mock.calls as unknown[][]).some((call) => + String(call[0]).includes("Continue anyway?"), + ); +} + +beforeEach(() => { + spies = []; + delete process.env.NEMOCLAW_NON_INTERACTIVE; + + logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); + errSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + exitMock = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code})`); + }) as never); + + // Registry seam. + getSandboxMock = vi.spyOn(registry, "getSandbox").mockReturnValue(null); + listSandboxesMock = vi + .spyOn(registry, "listSandboxes") + .mockReturnValue({ sandboxes: [], defaultSandbox: null }); + updateSandboxMock = vi.spyOn(registry, "updateSandbox").mockReturnValue(true); + + // onboard/providers seam (gateway probe + register). + vi.spyOn(providers, "providerExistsInGateway").mockReturnValue(false); + upsertMock = vi.spyOn(providers, "upsertMessagingProviders").mockImplementation(() => undefined); + + // openshell runtime + gateway recovery. + runOpenshellMock = vi + .spyOn(runtime, "runOpenshell") + .mockReturnValue({ status: 0, stdout: "", stderr: "" }); + vi.spyOn(gatewayRuntime, "recoverNamedGatewayRuntime").mockResolvedValue({ recovered: true }); + + // Credentials store: staged token (no real prompt) + controllable prompt. + getCredentialMock = vi.spyOn(store, "getCredential").mockReturnValue(null); + promptMock = vi.spyOn(store, "prompt").mockResolvedValue(""); + vi.spyOn(store, "saveCredential").mockImplementation(() => undefined); + + // Agent gate: support every channel. + vi.spyOn(defs, "loadAgent").mockReturnValue({ + name: "openclaw", + messagingPlatforms: ["telegram", "discord", "slack", "wechat", "whatsapp"], + }); + + // Policy seam. addSandboxChannel gates on loadPreset()/parsePresetPolicyKeys() + // up front (the channel must ship a preset with network_policies); stub both + // so the gate passes without reading preset YAML off disk. listPresets [] so + // no preset is treated as "built-in" for any channel + // (applyChannelPresetIfAvailable then short-circuits to success). + vi.spyOn(policy, "loadPreset").mockReturnValue("network_policies:\n stub: {}\n"); + vi.spyOn(policy, "parsePresetPolicyKeys").mockReturnValue(["stub"]); + vi.spyOn(policy, "listPresets").mockReturnValue([]); + applyPresetMock = vi.spyOn(policy, "applyPreset").mockReturnValue(true); + vi.spyOn(policy, "getAppliedPresets").mockReturnValue([]); + + // Downstream rebuild is not under test. + vi.spyOn(rebuild, "rebuildSandbox").mockResolvedValue(undefined); + + // After a successful interactive add, verifyChannelBridgeAfterRebuild probes + // the sandbox via executeSandboxExecCommand, which calls getOpenshellBinary() + // -> process.exit(1) when the openshell binary is absent (e.g. the CI + // unit-test runner; locally it is installed, so this only bites in CI). Stub + // the exec seam so the post-add verification never shells out and never trips + // the exit spy. The bridge verification is downstream and not under test here. + vi.spyOn(processRecovery, "executeSandboxExecCommand").mockReturnValue(null); + vi.spyOn(processRecovery, "executeSandboxCommand").mockReturnValue(null); + + // Slack add runs a credential validation (auth.test-style) before the + // conflict check; that is its own concern and would otherwise probe Slack. + // Stub it to pass so scenario 11 can reach the conflict logic. + vi.spyOn(slackValidation, "validateSlackChannelCredentials").mockReturnValue({ ok: true }); + + // onboard-session for the wechat host-qr branch. + vi.spyOn(onboardSession, "loadSession").mockReturnValue(null); + vi.spyOn(onboardSession, "updateSession").mockImplementation(() => undefined); +}); + +afterEach(() => { + vi.restoreAllMocks(); + for (const s of spies) s.mockRestore(); + delete process.env.NEMOCLAW_NON_INTERACTIVE; + delete process.env.WECHAT_ACCOUNT_ID; +}); + +describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { + // Scenario 1 + it("interactive matching-token conflict: warns, user continues, add proceeds", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + ], + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + promptMock.mockResolvedValue("y"); + + await addSandboxChannel("alpha", { channel: "telegram" }); + + const text = loggedText(); + expect(text).toContain("bob"); + expect(text).toContain("same telegram credential"); + expect(upsertMock).toHaveBeenCalledTimes(1); + expect(updateSandboxMock).toHaveBeenCalledWith("alpha", expect.any(Object)); + }); + + // Scenario 2 + it("interactive matching-token conflict: user aborts, nothing is mutated", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + ], + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + promptMock.mockResolvedValue("n"); + + await addSandboxChannel("alpha", { channel: "telegram" }); + + expect(loggedText()).toContain("same telegram credential"); + expect(upsertMock).not.toHaveBeenCalled(); + expect(updateSandboxMock).not.toHaveBeenCalledWith("alpha", expect.any(Object)); + expect(applyPresetMock).not.toHaveBeenCalled(); + }); + + it("interactive matching-token conflict: empty answer (default N) aborts", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + ], + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + promptMock.mockResolvedValue(""); // bare Enter -> default No + + await addSandboxChannel("alpha", { channel: "telegram" }); + + expect(upsertMock).not.toHaveBeenCalled(); + expect(updateSandboxMock).not.toHaveBeenCalledWith("alpha", expect.any(Object)); + }); + + // Scenario 3 + it("non-interactive matching-token conflict: aborts with exit(1) and guidance", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + ], + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + process.env.NEMOCLAW_NON_INTERACTIVE = "1"; + + await expect(addSandboxChannel("alpha", { channel: "telegram" })).rejects.toThrow( + "process.exit(1)", + ); + + expect(exitMock).toHaveBeenCalledWith(1); + const text = loggedText(); + expect(text).toContain("same telegram credential"); + expect(text).toContain("Aborting"); + expect(text).toContain("--force"); + expect(text).toContain("channels remove"); + expect(upsertMock).not.toHaveBeenCalled(); + expect(updateSandboxMock).not.toHaveBeenCalledWith("alpha", expect.any(Object)); + expect(promptMock).not.toHaveBeenCalled(); + }); + + // Scenario 4 + it("--force bypasses the conflict even in non-interactive mode", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + ], + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + process.env.NEMOCLAW_NON_INTERACTIVE = "1"; + + await addSandboxChannel("alpha", { channel: "telegram", force: true }); + + const text = loggedText(); + expect(text).toContain("same telegram credential"); // warning still shown + expect(text).toContain("--force"); // proceed line + expect(exitMock).not.toHaveBeenCalled(); + expect(upsertMock).toHaveBeenCalledTimes(1); + expect(updateSandboxMock).toHaveBeenCalledWith("alpha", expect.any(Object)); + expect(promptMock).not.toHaveBeenCalled(); + }); + + // Scenario 5a + it("unknown-token wording when the other sandbox has the channel but no hash", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [{ name: "bob", messagingChannels: ["telegram"] }], // no providerCredentialHashes + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + promptMock.mockResolvedValue("y"); + + await addSandboxChannel("alpha", { channel: "telegram" }); + + const text = loggedText(); + expect(text).toContain("credential hash is unavailable"); + expect(text).not.toContain("same telegram credential"); + }); + + // Scenario 5b + it("different hash on the other sandbox is NOT a conflict (no warning, add proceeds)", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { + TELEGRAM_BOT_TOKEN: hashCredential("a-completely-different-token") as string, + }, + }, + ], + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + promptMock.mockResolvedValue("n"); // would abort IF prompted; proves no prompt happens + + await addSandboxChannel("alpha", { channel: "telegram" }); + + const text = loggedText(); + expect(text).not.toContain("credential hash is unavailable"); + expect(text).not.toContain("same telegram credential"); + expect(conflictPromptShown()).toBe(false); + expect(upsertMock).toHaveBeenCalledTimes(1); + expect(updateSandboxMock).toHaveBeenCalledWith("alpha", expect.any(Object)); + }); + + // Scenario 6 + it("idempotent same-sandbox re-add does not self-conflict", async () => { + arrangeRegistry({ + current: { + name: "alpha", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + promptMock.mockResolvedValue("n"); // would abort IF prompted + + await addSandboxChannel("alpha", { channel: "telegram" }); + + const text = loggedText(); + expect(text).not.toContain("same telegram credential"); + expect(text).not.toContain("credential hash is unavailable"); + expect(conflictPromptShown()).toBe(false); + expect(upsertMock).toHaveBeenCalledTimes(1); + expect(updateSandboxMock).toHaveBeenCalledWith("alpha", expect.any(Object)); + }); + + // Scenario 7 + it("--dry-run never runs the conflict check or touches credentials", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + ], + }); + + await addSandboxChannel("alpha", { channel: "telegram", dryRun: true }); + + const text = loggedText(); + expect(text).toContain("--dry-run: would enable channel 'telegram'"); + expect(text).not.toContain("same telegram credential"); + expect(text).not.toContain("credential hash is unavailable"); + expect(getCredentialMock).not.toHaveBeenCalled(); + expect(runOpenshellMock).not.toHaveBeenCalled(); + expect(upsertMock).not.toHaveBeenCalled(); + expect(promptMock).not.toHaveBeenCalled(); + expect(exitMock).not.toHaveBeenCalled(); + }); + + // Scenario 8: WeChat is host-qr (token-bearing) -> non-empty acquired -> IS conflict-checked. + it("host-qr wechat (token-bearing) IS conflict-checked", async () => { + const wechatToken = "wx-secret-token-abc"; + const wechatHash = hashCredential(wechatToken) as string; + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["wechat"], + providerCredentialHashes: { WECHAT_BOT_TOKEN: wechatHash }, + }, + ], + }); + // acquireHostQrChannel short-circuits on a cached token; supply account + // metadata so the wechat cached-token branch does not bail. + getCredentialMock.mockImplementation((key: string) => + key === "WECHAT_BOT_TOKEN" ? wechatToken : null, + ); + process.env.WECHAT_ACCOUNT_ID = "acct-1"; + process.env.NEMOCLAW_NON_INTERACTIVE = "1"; + + await expect(addSandboxChannel("alpha", { channel: "wechat" })).rejects.toThrow( + "process.exit(1)", + ); + expect(loggedText()).toContain("same wechat credential"); + expect(exitMock).toHaveBeenCalledWith(1); + }); + + // Scenario 8 (companion): genuinely in-sandbox-QR channel (whatsapp) has empty + // acquired and skips the credential conflict check entirely. + it("in-sandbox-qr whatsapp skips the credential conflict check", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["whatsapp"], + providerCredentialHashes: { WHATSAPP_TOKEN: "irrelevant" }, + }, + ], + }); + process.env.NEMOCLAW_NON_INTERACTIVE = "1"; + + await addSandboxChannel("alpha", { channel: "whatsapp" }); + + const text = loggedText(); + expect(text).not.toContain("Continue anyway?"); + expect(text).toContain("Enabled whatsapp channel"); + expect(exitMock).not.toHaveBeenCalled(); + expect(promptMock).not.toHaveBeenCalled(); + }); + + // Scenario 9 + it("probe + backfill failure is swallowed; a pre-recorded matching hash still warns", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + // Legacy entry with NO messagingChannels field — backfill probes the + // (alive) gateway, gets "absent" for every provider, then writes + // messagingChannels:[] for it. We make THAT write throw to genuinely + // exercise the try/catch around backfillMessagingChannels. + { name: "legacy" }, + ], + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + // Gateway alive (status 0) + every provider absent, so backfill reaches the + // updateSandbox("legacy") write — which throws below. + runOpenshellMock.mockReturnValue({ status: 0, stdout: "", stderr: "" }); + updateSandboxMock.mockImplementation((name: string, _updates: Partial) => { + if (name === "legacy") throw new Error("backfill boom"); + return true; + }); + process.env.NEMOCLAW_NON_INTERACTIVE = "1"; + + // bob already has messagingChannels + a matching hash, so the conflict is + // still found -> non-interactive abort. Key guarantee: a throw inside + // backfillMessagingChannels is swallowed; the only exit is the conflict + // exit(1), not an unhandled exception. + await expect(addSandboxChannel("alpha", { channel: "telegram" })).rejects.toThrow( + "process.exit(1)", + ); + expect(exitMock).toHaveBeenCalledWith(1); + expect(loggedText()).toContain("same telegram credential"); + }); + + it("probe + backfill failure with no pre-recorded conflict lets the add proceed", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [], // no other sandbox -> no conflict resolvable + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + runOpenshellMock.mockReturnValue({ status: 1, stdout: "", stderr: "down" }); + + await addSandboxChannel("alpha", { channel: "telegram" }); + + expect(exitMock).not.toHaveBeenCalled(); + expect(upsertMock).toHaveBeenCalledTimes(1); + expect(updateSandboxMock).toHaveBeenCalledWith("alpha", expect.any(Object)); + }); + + // Scenario 10 + it("never prints the raw token value in any conflict output (proceed path)", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + ], + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + promptMock.mockResolvedValue("y"); + + await addSandboxChannel("alpha", { channel: "telegram" }); + + const text = loggedText(); + expect(text).toContain("same telegram credential"); // sanity + expect(text).not.toContain(TELEGRAM_TOKEN); // no raw secret + expect(text).not.toContain(TELEGRAM_HASH); // hash not in conflict warning text + }); + + it("non-interactive abort path also keeps the raw token out of output", async () => { + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["telegram"], + providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, + }, + ], + }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + process.env.NEMOCLAW_NON_INTERACTIVE = "1"; + + await expect(addSandboxChannel("alpha", { channel: "telegram" })).rejects.toThrow( + "process.exit(1)", + ); + + expect(loggedText()).not.toContain(TELEGRAM_TOKEN); + }); + + // Scenario 11 + it("slack two-token channel: matching SLACK_BOT_TOKEN hash is detected", async () => { + const slackBot = "xoxb-1234-5678-abcdef"; + const slackApp = "xapp-1-A0000-12345-abcdef"; + const slackBotHash = hashCredential(slackBot) as string; + arrangeRegistry({ + current: { name: "alpha", messagingChannels: [] }, + others: [ + { + name: "bob", + messagingChannels: ["slack"], + providerCredentialHashes: { SLACK_BOT_TOKEN: slackBotHash }, // only bot token matches + }, + ], + }); + getCredentialMock.mockImplementation((key: string) => { + if (key === "SLACK_BOT_TOKEN") return slackBot; + if (key === "SLACK_APP_TOKEN") return slackApp; + return null; + }); + promptMock.mockResolvedValue("y"); + + await addSandboxChannel("alpha", { channel: "slack" }); + + const text = loggedText(); + expect(text).toContain("bob"); + expect(text).toContain("same slack credential"); // matching-token wording + expect(text).not.toContain(slackBot); + expect(text).not.toContain(slackApp); + expect(upsertMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index d3f3fbc6b7..1d90e0937f 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -5,50 +5,54 @@ import fs from "node:fs"; import path from "node:path"; -import { loadAgent, type AgentDefinition } from "../../agent/defs"; +import { type AgentDefinition, loadAgent } from "../../agent/defs"; import { CLI_DISPLAY_NAME, CLI_NAME } from "../../cli/branding"; -import { hashCredential } from "../../security/credential-hash"; -import { getCredential, prompt as askPrompt } from "../../credentials/store"; +import { prompt as askPrompt, getCredential } from "../../credentials/store"; import { recoverNamedGatewayRuntime } from "../../gateway-runtime-action"; +import { hashCredential } from "../../security/credential-hash"; + const { isNonInteractive } = require("../../onboard") as { isNonInteractive: () => boolean }; const onboardProviders = require("../../onboard/providers"); + import * as policies from "../../policy"; + // Lazy-required: keeps qrcode-terminal + the iLink HTTP client out of the // import graph for non-host-qr channels-add calls. const { HOST_QR_LOGIN_HANDLERS } = require("../../host-qr-handlers") as typeof import("../../host-qr-handlers"); const onboardSession = require("../../state/onboard-session") as typeof import("../../state/onboard-session"); +import { runOpenshell } from "../../adapters/openshell/runtime"; import { - parsePolicyAddOptions, type PolicyAddOptions, type PolicyRemoveOptions, + parsePolicyAddOptions, } from "../../domain/policy-channel"; -import * as registry from "../../state/registry"; -import { runOpenshell } from "../../adapters/openshell/runtime"; +import type { HostQrLoginResult } from "../../host-qr-handlers"; import { shellQuote } from "../../runner"; -import { - isDockerRuntimeDown, - printDockerRuntimeDownGuidance, -} from "./gateway-failure-classifier"; -import { executeSandboxCommand, executeSandboxExecCommand } from "./process-recovery"; -import { rebuildSandbox } from "./rebuild"; -import { validateSlackChannelCredentials } from "./slack-channel-validation"; -import { printTelegramDirectMessageAllowlistWarning } from "./telegram-channel-bridge-verification"; import { type ChannelDef, - KNOWN_CHANNELS, channelUsesInSandboxQrPairing, clearChannelTokens, getChannelDef, getChannelTokenKeys, + KNOWN_CHANNELS, knownChannelNames, persistChannelTokens, } from "../../sandbox/channels"; -import type { HostQrLoginResult } from "../../host-qr-handlers"; +import * as registry from "../../state/registry"; +import { + isDockerRuntimeDown, + printDockerRuntimeDownGuidance, +} from "./gateway-failure-classifier"; +import { executeSandboxCommand, executeSandboxExecCommand } from "./process-recovery"; +import { rebuildSandbox } from "./rebuild"; +import { validateSlackChannelCredentials } from "./slack-channel-validation"; +import { printTelegramDirectMessageAllowlistWarning } from "./telegram-channel-bridge-verification"; type ChannelMutationOptions = { channel?: string; dryRun?: boolean; + force?: boolean; }; const useColor = !process.env.NO_COLOR && !!process.stdout.isTTY; @@ -323,6 +327,98 @@ function bridgeProviderName(sandboxName: string, channelName: string, envKey: st return `${sandboxName}-${channelName}-bridge`; } +// Tri-state gateway probe for cross-sandbox messaging-conflict backfill, +// mirroring onboard.ts makeConflictProbe(). An upfront liveness check keeps a +// transient gateway failure ("error") from being mis-recorded as "no +// providers" ("absent"), which would permanently suppress backfill retries. +function makeChannelsConflictProbe() { + let gatewayAlive: boolean | null = null; + const isGatewayAlive = (): boolean => { + if (gatewayAlive === null) { + const result = runOpenshell(["sandbox", "list"], { + ignoreError: true, + stdio: ["ignore", "ignore", "ignore"], + }); + gatewayAlive = result.status === 0; + } + return gatewayAlive; + }; + return { + providerExists: (name: string): "present" | "absent" | "error" => { + if (!isGatewayAlive()) return "error"; + return onboardProviders.providerExistsInGateway(name, runOpenshell) ? "present" : "absent"; + }, + }; +} + +// Detect whether another sandbox already uses one of this channel's +// credentials. Mirrors the onboard.ts conflict check. Returns true if the +// caller should PROCEED with the add, false if it should abort. Never logs +// credential values — only the non-secret hashes computed inline are passed +// to findChannelConflicts. Probe/backfill failures are swallowed +// (non-fatal): backfillMessagingChannels already skips sandboxes whose probe +// errors, so a flaky gateway degrades to "no conflict found" rather than +// blocking the add. +async function checkChannelAddConflict( + sandboxName: string, + channelName: string, + acquired: Record, + force: boolean, +): Promise { + // QR-paired / tokenless adds have empty `acquired` and no host-side + // credential to hash. Skip — there is no credential to collide on, and + // findChannelConflicts with empty credentialHashes would only ever report + // "unknown-token" noise against every other sandbox holding the channel. + const credentialHashes: Record = {}; + for (const [envKey, token] of Object.entries(acquired)) { + const hash = hashCredential(token); + if (hash) credentialHashes[envKey] = hash; + } + if (Object.keys(credentialHashes).length === 0) return true; + + const { backfillMessagingChannels, findChannelConflicts } = + require("../../messaging-conflict") as typeof import("../../messaging-conflict"); + + try { + backfillMessagingChannels(registry, makeChannelsConflictProbe()); + } catch { + // Non-fatal: a backfill blow-up must not block adding a channel. + } + + let conflicts: ReturnType; + try { + conflicts = findChannelConflicts(sandboxName, [{ channel: channelName, credentialHashes }], registry); + } catch { + return true; + } + if (conflicts.length === 0) return true; + + for (const { channel, sandbox, reason } of conflicts) { + const detail = + reason === "matching-token" + ? `uses the same ${channel} credential` + : `already has ${channel} enabled, but its credential hash is unavailable`; + console.log( + ` ${YW}⚠${R} Sandbox '${sandbox}' ${detail}. Shared channel credentials only allow one sandbox to poll/connect — continuing may break both bridges (e.g. Telegram getUpdates 409).`, + ); + } + + if (force) { + console.log(` --force: proceeding despite the messaging channel conflict above.`); + return true; + } + if (isNonInteractive()) { + console.error( + ` Aborting: resolve the messaging channel conflict above, run \`${CLI_NAME} channels remove ${channelName}\` on the other sandbox, or re-run with --force.`, + ); + process.exit(1); + } + const answer = (await askPrompt(" Continue anyway? [y/N]: ")).trim().toLowerCase(); + if (answer === "y" || answer === "yes") return true; + console.log(" Aborting channel add."); + return false; +} + // Push channel tokens to the OpenShell gateway and add the channel to the // sandbox registry's messagingChannels list. Done eagerly at `channels // add` time (not deferred to rebuild) because the host-side credential @@ -799,6 +895,7 @@ export async function addSandboxChannel( options: ChannelMutationOptions = {}, ): Promise { const dryRun = Boolean(options.dryRun); + const force = Boolean(options.force); const rawChannelArg = options.channel; if (!rawChannelArg) { console.error(` Usage: ${CLI_NAME} channels add [--dry-run]`); @@ -901,6 +998,9 @@ export async function addSandboxChannel( } persistChannelTokens(acquired); + if (!(await checkChannelAddConflict(sandboxName, canonical, acquired, force))) { + return; // user aborted; nothing registered or widened + } // Push to the gateway and update the registry NOW so that answering // "rebuild later" (or running non-interactively) does not silently // discard the change. Pre-fix this was safe because saveCredential() diff --git a/src/lib/sandbox/channels-command-support.ts b/src/lib/sandbox/channels-command-support.ts index a8a5ce3fb0..03bc82eca7 100644 --- a/src/lib/sandbox/channels-command-support.ts +++ b/src/lib/sandbox/channels-command-support.ts @@ -3,11 +3,12 @@ import { Args } from "@oclif/core"; -import { dryRunFlag } from "../cli/common-flags"; +import { dryRunFlag, forceFlag } from "../cli/common-flags"; export type ChannelMutationOptions = { channel?: string; dryRun?: boolean; + force?: boolean; }; const sandboxNameArg = Args.string({ name: "sandbox", description: "Sandbox name", required: true }); @@ -15,11 +16,12 @@ const channelArg = Args.string({ name: "channel", description: "Messaging channe export function channelMutationOptions( channel: string | undefined, - flags: { "dry-run"?: boolean }, + flags: { "dry-run"?: boolean; force?: boolean }, ): ChannelMutationOptions { return { channel, dryRun: Boolean(flags["dry-run"]), + force: Boolean(flags.force), }; } @@ -31,3 +33,12 @@ export const channelMutationArgs = { export const channelMutationFlags = { "dry-run": dryRunFlag("Preview the change without applying it"), }; + +// `--force` is add-only: only `channels add` can overlap a messaging +// credential another sandbox already uses, so only it exposes the override. +// Keeping it off remove/start/stop avoids a misleading no-op flag and keeps +// CLI/docs flag parity (the shared object would surface --force everywhere). +export const channelAddFlags = { + ...channelMutationFlags, + force: forceFlag("Add the channel even if another sandbox already uses this credential"), +};