diff --git a/.changeset/clarify-channel-default-enable-guidance.md b/.changeset/clarify-channel-default-enable-guidance.md new file mode 100644 index 00000000..bfa89fbf --- /dev/null +++ b/.changeset/clarify-channel-default-enable-guidance.md @@ -0,0 +1,5 @@ +--- +"@plotday/twister": patch +--- + +Changed: clarified the `Channel.enabledByDefault` doc comment to recommend an explicit "is this the user's own resource" signal over an ACL/permission-tier check when deciding which channels to sync by default — a high permission tier can be granted on a resource the user doesn't own (e.g. broad internal sharing defaults), so it doesn't reliably mean "mine". diff --git a/connectors/google/src/calendar/channels.test.ts b/connectors/google/src/calendar/channels.test.ts new file mode 100644 index 00000000..3cbbb4d9 --- /dev/null +++ b/connectors/google/src/calendar/channels.test.ts @@ -0,0 +1,84 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import type { AuthToken } from "@plotday/twister/tools/integrations"; + +import { CALENDAR_LIST_SCOPE, getCalendarChannels } from "./channels"; + +function stubCalendarList( + items: Array<{ + id: string; + summary: string; + primary?: boolean; + accessRole?: string; + }> +) { + const spy = vi.fn( + async () => + new Response(JSON.stringify({ items }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }) + ); + vi.stubGlobal("fetch", spy); + return spy; +} + +function token(scopes: string[] = [CALENDAR_LIST_SCOPE]): AuthToken { + return { token: "test-token", scopes }; +} + +describe("getCalendarChannels", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("enables only the user's own primary calendar by default", async () => { + stubCalendarList([ + { id: "erin@artemiscanada.com", summary: "erin@artemiscanada.com", primary: true, accessRole: "owner" }, + ]); + + const channels = await getCalendarChannels(token()); + + expect(channels).toEqual([ + { id: "erin@artemiscanada.com", title: "erin@artemiscanada.com", enabledByDefault: true }, + ]); + }); + + it("does NOT enable a colleague's calendar shared at owner-level ACL", async () => { + // Regression: Workspace domains commonly grant "owner"-tier internal + // sharing on every user's calendar. accessRole === "owner" alone must + // not be treated as "this is my calendar" — only `primary` may. + stubCalendarList([ + { id: "erin@artemiscanada.com", summary: "erin@artemiscanada.com", primary: true, accessRole: "owner" }, + { id: "alex@artemiscanada.com", summary: "alex@artemiscanada.com", primary: false, accessRole: "owner" }, + ]); + + const channels = await getCalendarChannels(token()); + + expect(channels).toEqual([ + { id: "erin@artemiscanada.com", title: "erin@artemiscanada.com", enabledByDefault: true }, + { id: "alex@artemiscanada.com", title: "alex@artemiscanada.com", enabledByDefault: false }, + ]); + }); + + it("does not enable a non-owned shared or subscribed calendar", async () => { + stubCalendarList([ + { id: "erin@artemiscanada.com", summary: "erin@artemiscanada.com", primary: true, accessRole: "owner" }, + { id: "en.canadian#holiday@group.v.calendar.google.com", summary: "Holidays in Canada", primary: false, accessRole: "reader" }, + ]); + + const channels = await getCalendarChannels(token()); + + expect(channels.find((c) => c.title === "Holidays in Canada")).toEqual({ + id: "en.canadian#holiday@group.v.calendar.google.com", + title: "Holidays in Canada", + enabledByDefault: false, + }); + }); + + it("returns a single enabled 'primary' fallback channel without CALENDAR_LIST_SCOPE", async () => { + const channels = await getCalendarChannels(token([])); + + expect(channels).toEqual([{ id: "primary", title: "Calendar", enabledByDefault: true }]); + }); +}); diff --git a/connectors/google/src/calendar/channels.ts b/connectors/google/src/calendar/channels.ts index 0eb47612..8a7bf4ad 100644 --- a/connectors/google/src/calendar/channels.ts +++ b/connectors/google/src/calendar/channels.ts @@ -22,13 +22,20 @@ export type Calendar = { id: string; name: string; description: string | null; + /** + * True for exactly one calendar per calendarList: the authenticated + * user's own default calendar (id === their email). Drives the + * default-enable decision in getChannels. + */ primary: boolean; /** - * The user's access level on this calendar: "owner", "writer", "reader", - * or "freeBusyReader". Calendars the user owns (their primary + any - * secondary calendars they created) are "owner"; subscribed holiday/ - * birthday calendars and someone-else's shared calendars are "reader"/ - * "writer". Drives the default-enable decision in getChannels. + * The user's ACL permission tier on this calendar: "owner", "writer", + * "reader", or "freeBusyReader". This is a *permission level*, not an + * ownership signal — a domain admin or a colleague can grant "owner" + * (full manage-and-share access) on a calendar they don't personally + * use, e.g. Workspace-wide internal sharing defaults or a teammate + * sharing their own calendar for scheduling coverage. Do not use this + * to infer "is this the user's own calendar" — use `primary` instead. */ accessRole: string | null; }; @@ -101,7 +108,15 @@ export async function listCalendars(api: GoogleApi): Promise { * - If CALENDAR_LIST_SCOPE is absent, returns a single "primary" fallback * channel (avoids a 403 from calling calendarList without the scope). * - Otherwise calls the calendarList API and maps each calendar to a channel, - * defaulting to enabled only for calendars the user owns (accessRole "owner"). + * defaulting to enabled only for the user's own primary calendar. + * + * Deliberately does NOT use `accessRole === "owner"` for this: that ACL tier + * is granted to anyone with full manage-and-share access on a calendar, not + * just its actual owner. Workspace domains commonly default internal + * calendar sharing to "owner", or teammates share their own calendars with + * each other for scheduling coverage — either way, every other user's + * personal calendar would also read `accessRole: "owner"` and get swept + * into the default-enabled set alongside the user's own. */ export async function getCalendarChannels(token: AuthToken): Promise { if (!token.scopes.includes(CALENDAR_LIST_SCOPE)) { @@ -112,6 +127,6 @@ export async function getCalendarChannels(token: AuthToken): Promise return calendars.map((c) => ({ id: c.id, title: c.name, - enabledByDefault: c.accessRole === "owner", + enabledByDefault: c.primary, })); } diff --git a/twister/src/tools/integrations.ts b/twister/src/tools/integrations.ts index 4a1ef444..c018e29d 100644 --- a/twister/src/tools/integrations.ts +++ b/twister/src/tools/integrations.ts @@ -36,7 +36,10 @@ export type Channel = { * The guiding principle is "sync everything the user would reasonably want * by default" — for most connectors that's all channels, so only set this * where the connector can distinguish the user's own/relevant channels from - * low-value ones (e.g. Google Calendar via `accessRole === "owner"`). + * low-value ones (e.g. Google Calendar via its `primary` flag). Prefer an + * explicit "is this the user's own resource" signal over an ACL/permission + * tier — a high permission level (e.g. "owner"-tier sharing) can be granted + * on someone else's resource too, so it does not reliably mean "mine". */ enabledByDefault?: boolean; /** Optional nested channel resources (e.g., subfolders) */