From ff19a7bbce04284d2bce4f1cd2e2164c0bf72e0e Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Sat, 31 Oct 2026 13:48:33 -0700 Subject: [PATCH 1/2] fix(cli): refuse event triggers whose integration the owner has not connected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Linear and the other user-scoped connectors only deliver an event to the automations of the member who connected them. `automations create` and `update` saved such a trigger without checking, so an automation made from the CLI or by an agent could never fire and nothing said why. The desktop editor already flags this as "Requires connection". Check `integration.connectionStatus` before saving and fail with the fix: connect (or reconnect) the integration in Settings → Integrations, then rerun. Triggers that need no connection (schedule, webhook, GitHub) are never refused, and a failed status check does not block the save. Fixes SUPER-3063 --- .../commands/automations/create/command.ts | 3 ++ .../missingConnectionError.test.ts | 39 ++++++++++++++ .../automations/missingConnectionError.ts | 51 +++++++++++++++++++ .../commands/automations/update/command.ts | 7 +++ 4 files changed, 100 insertions(+) create mode 100644 packages/cli/src/commands/automations/missingConnectionError.test.ts create mode 100644 packages/cli/src/commands/automations/missingConnectionError.ts diff --git a/packages/cli/src/commands/automations/create/command.ts b/packages/cli/src/commands/automations/create/command.ts index ce8ee5da084..dc516741fa6 100644 --- a/packages/cli/src/commands/automations/create/command.ts +++ b/packages/cli/src/commands/automations/create/command.ts @@ -3,6 +3,7 @@ import { boolean, CLIError, string } from "@superset/cli-framework"; import { command } from "../../../lib/command"; import { resolveHostFilter } from "../../../lib/host-target"; import { formatAutomationDate } from "../format"; +import { assertTriggersConnected } from "../missingConnectionError"; import { resolveAutomationTarget } from "../resolveAutomationTarget"; import { refuseHostFlagsForCloud, @@ -115,6 +116,8 @@ export default command({ v2WorkspaceId: options.workspace ?? undefined, }; + await assertTriggersConnected(ctx.api, organizationId, triggers); + const result = await ctx.api.automation.create.mutate({ name: options.name, prompt, diff --git a/packages/cli/src/commands/automations/missingConnectionError.test.ts b/packages/cli/src/commands/automations/missingConnectionError.test.ts new file mode 100644 index 00000000000..0f8340de22f --- /dev/null +++ b/packages/cli/src/commands/automations/missingConnectionError.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "bun:test"; +import { missingConnectionError } from "./missingConnectionError"; + +const linear = { config: { kind: "linear" } }; +const slack = { config: { kind: "slack" } }; +const live = { connected: true, needsReauth: false }; + +describe("missingConnectionError", () => { + it("refuses a trigger whose integration the caller has not connected", () => { + const error = missingConnectionError([linear], {}); + expect(error?.message).toContain("you have not connected Linear"); + expect(error?.suggestion).toContain("Connect Linear in Settings"); + }); + + it("asks for a reconnect when the connection expired", () => { + const error = missingConnectionError([linear], { + linear: { connected: false, needsReauth: true }, + }); + expect(error?.suggestion).toContain("Reconnect Linear"); + }); + + it("allows a trigger whose integration is connected", () => { + expect(missingConnectionError([linear], { linear: live })).toBeNull(); + }); + + it("names every missing integration once", () => { + const error = missingConnectionError([linear, linear, slack], {}); + expect(error?.message).toContain("Linear, Slack"); + }); + + it("ignores triggers that need no connection", () => { + expect( + missingConnectionError( + [{ config: { kind: "schedule" } }, { config: { kind: "github" } }], + {}, + ), + ).toBeNull(); + }); +}); diff --git a/packages/cli/src/commands/automations/missingConnectionError.ts b/packages/cli/src/commands/automations/missingConnectionError.ts new file mode 100644 index 00000000000..17bd04d3df9 --- /dev/null +++ b/packages/cli/src/commands/automations/missingConnectionError.ts @@ -0,0 +1,51 @@ +import { CLIError } from "@superset/cli-framework"; +import { TRIGGER_KIND_CONNECTOR } from "@superset/shared/automation-triggers"; +import { getConnector } from "@superset/shared/connectors"; +import type { ApiClient } from "../../lib/api-client"; + +export interface ConnectionState { + connected: boolean; + needsReauth: boolean; +} + +export function missingConnectionError( + triggers: { config: { kind: string } }[], + status: Record, +): CLIError | null { + const connectors = new Set( + triggers.flatMap(({ config }) => { + const connector = TRIGGER_KIND_CONNECTOR[config.kind]; + return connector ? [connector] : []; + }), + ); + const missing = [...connectors].filter( + (connector) => !status[connector]?.connected, + ); + if (missing.length === 0) return null; + + const name = (connector: string) => + getConnector(connector)?.displayName ?? connector; + const steps = missing.map((connector) => + status[connector]?.needsReauth + ? `Your ${name(connector)} connection expired. Reconnect ${name(connector)} in Settings → Integrations.` + : `Connect ${name(connector)} in Settings → Integrations.`, + ); + return new CLIError( + `These triggers would never fire: you have not connected ${missing.map(name).join(", ")}`, + [...steps, "Then run this command again."].join(" "), + ); +} + +export async function assertTriggersConnected( + api: ApiClient, + organizationId: string | undefined, + triggers: { config: { kind: string } }[] | undefined, +): Promise { + if (!organizationId || !triggers?.length) return; + const status = await api.integration.connectionStatus + .query({ organizationId }) + .catch(() => null); + if (!status) return; + const error = missingConnectionError(triggers, status); + if (error) throw error; +} diff --git a/packages/cli/src/commands/automations/update/command.ts b/packages/cli/src/commands/automations/update/command.ts index e9e1d91ba9b..28622dc48b6 100644 --- a/packages/cli/src/commands/automations/update/command.ts +++ b/packages/cli/src/commands/automations/update/command.ts @@ -2,6 +2,7 @@ import { boolean, CLIError, positional, string } from "@superset/cli-framework"; import { CLOUD_HOST_ID } from "@superset/shared/host-routing"; import { command } from "../../../lib/command"; import { resolveHostFilter } from "../../../lib/host-target"; +import { assertTriggersConnected } from "../missingConnectionError"; import { resolveAutomationTarget } from "../resolveAutomationTarget"; import { refuseHostFlagsForCloud, @@ -86,6 +87,12 @@ export default command({ const triggers = resolveTriggers(options); + await assertTriggersConnected( + ctx.api, + ctx.config.organizationId ?? undefined, + triggers, + ); + const targetHostId = resolveHostFilter({ host: options.host ?? undefined, local: options.local ?? undefined, From edbc37a0ac515e571314a4d47cedb86d3c1efadd Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Sat, 31 Oct 2026 14:19:51 -0700 Subject: [PATCH 2/2] fix(cli): check a trigger's pinned account and word expired connections apart A trigger pinned to an expired account passed while another account on the same connector was live, and a pin to an account the caller does not own passed too. Check the pinned account itself. An expired connection now says so in the headline instead of "you have not connected". Also covers the fail-open path of assertTriggersConnected. --- .../missingConnectionError.test.ts | 67 ++++++++++++++++- .../automations/missingConnectionError.ts | 71 ++++++++++++++----- 2 files changed, 118 insertions(+), 20 deletions(-) diff --git a/packages/cli/src/commands/automations/missingConnectionError.test.ts b/packages/cli/src/commands/automations/missingConnectionError.test.ts index 0f8340de22f..5ac0d91e366 100644 --- a/packages/cli/src/commands/automations/missingConnectionError.test.ts +++ b/packages/cli/src/commands/automations/missingConnectionError.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "bun:test"; -import { missingConnectionError } from "./missingConnectionError"; +import type { ApiClient } from "../../lib/api-client"; +import { + assertTriggersConnected, + missingConnectionError, +} from "./missingConnectionError"; const linear = { config: { kind: "linear" } }; const slack = { config: { kind: "slack" } }; @@ -16,16 +20,48 @@ describe("missingConnectionError", () => { const error = missingConnectionError([linear], { linear: { connected: false, needsReauth: true }, }); + expect(error?.message).toContain("your Linear connection expired"); expect(error?.suggestion).toContain("Reconnect Linear"); }); + it("refuses a pin to an expired account while another account is live", () => { + const error = missingConnectionError([{ ...linear, connectionId: "old" }], { + linear: { + ...live, + accounts: [ + { id: "old", needsReauth: true }, + { id: "new", needsReauth: false }, + ], + }, + }); + expect(error?.message).toContain("your Linear connection expired"); + }); + + it("refuses a pin to an account the caller does not own", () => { + const error = missingConnectionError( + [{ ...linear, connectionId: "theirs" }], + { linear: { ...live, accounts: [{ id: "mine", needsReauth: false }] } }, + ); + expect(error?.message).toContain("pinned to an account"); + }); + + it("allows a pin to a live account", () => { + expect( + missingConnectionError([{ ...linear, connectionId: "mine" }], { + linear: { ...live, accounts: [{ id: "mine", needsReauth: false }] }, + }), + ).toBeNull(); + }); + it("allows a trigger whose integration is connected", () => { expect(missingConnectionError([linear], { linear: live })).toBeNull(); }); it("names every missing integration once", () => { const error = missingConnectionError([linear, linear, slack], {}); - expect(error?.message).toContain("Linear, Slack"); + expect(error?.message).toBe( + "These triggers would never fire: you have not connected Linear; you have not connected Slack", + ); }); it("ignores triggers that need no connection", () => { @@ -37,3 +73,30 @@ describe("missingConnectionError", () => { ).toBeNull(); }); }); + +describe("assertTriggersConnected", () => { + const api = (query: () => Promise) => + ({ integration: { connectionStatus: { query } } }) as unknown as ApiClient; + + it("throws when the status names a missing connection", async () => { + await expect( + assertTriggersConnected( + api(async () => ({})), + "org", + [linear], + ), + ).rejects.toThrow("you have not connected Linear"); + }); + + it("lets the save go ahead when the status query fails", async () => { + await expect( + assertTriggersConnected( + api(async () => { + throw new Error("offline"); + }), + "org", + [linear], + ), + ).resolves.toBeUndefined(); + }); +}); diff --git a/packages/cli/src/commands/automations/missingConnectionError.ts b/packages/cli/src/commands/automations/missingConnectionError.ts index 17bd04d3df9..6411dcacd00 100644 --- a/packages/cli/src/commands/automations/missingConnectionError.ts +++ b/packages/cli/src/commands/automations/missingConnectionError.ts @@ -6,32 +6,67 @@ import type { ApiClient } from "../../lib/api-client"; export interface ConnectionState { connected: boolean; needsReauth: boolean; + accounts?: { id: string; needsReauth: boolean }[]; +} + +type Problem = "missing" | "expired" | "foreignPin"; + +interface TriggerInput { + config: { kind: string }; + connectionId?: string | null; +} + +function problemFor( + trigger: TriggerInput, + state: ConnectionState | undefined, +): Problem | null { + if (trigger.connectionId) { + const account = state?.accounts?.find( + (candidate) => candidate.id === trigger.connectionId, + ); + if (!account) return "foreignPin"; + return account.needsReauth ? "expired" : null; + } + if (state?.connected) return null; + return state?.needsReauth ? "expired" : "missing"; } export function missingConnectionError( - triggers: { config: { kind: string } }[], + triggers: TriggerInput[], status: Record, ): CLIError | null { - const connectors = new Set( - triggers.flatMap(({ config }) => { - const connector = TRIGGER_KIND_CONNECTOR[config.kind]; - return connector ? [connector] : []; - }), - ); - const missing = [...connectors].filter( - (connector) => !status[connector]?.connected, - ); - if (missing.length === 0) return null; + const problems = new Map(); + for (const trigger of triggers) { + const connector = TRIGGER_KIND_CONNECTOR[trigger.config.kind]; + if (!connector || problems.has(connector)) continue; + const problem = problemFor(trigger, status[connector]); + if (problem) problems.set(connector, problem); + } + if (problems.size === 0) return null; const name = (connector: string) => getConnector(connector)?.displayName ?? connector; - const steps = missing.map((connector) => - status[connector]?.needsReauth - ? `Your ${name(connector)} connection expired. Reconnect ${name(connector)} in Settings → Integrations.` - : `Connect ${name(connector)} in Settings → Integrations.`, - ); + const reasons: string[] = []; + const steps: string[] = []; + for (const [connector, problem] of problems) { + const n = name(connector); + if (problem === "expired") { + reasons.push(`your ${n} connection expired`); + steps.push(`Reconnect ${n} in Settings → Integrations.`); + } else if (problem === "foreignPin") { + reasons.push( + `a ${n} trigger is pinned to an account you have not connected`, + ); + steps.push( + `Drop its connectionId, or set it to one of your ${n} accounts.`, + ); + } else { + reasons.push(`you have not connected ${n}`); + steps.push(`Connect ${n} in Settings → Integrations.`); + } + } return new CLIError( - `These triggers would never fire: you have not connected ${missing.map(name).join(", ")}`, + `These triggers would never fire: ${reasons.join("; ")}`, [...steps, "Then run this command again."].join(" "), ); } @@ -39,7 +74,7 @@ export function missingConnectionError( export async function assertTriggersConnected( api: ApiClient, organizationId: string | undefined, - triggers: { config: { kind: string } }[] | undefined, + triggers: TriggerInput[] | undefined, ): Promise { if (!organizationId || !triggers?.length) return; const status = await api.integration.connectionStatus