diff --git a/src/slack/identity.ts b/src/slack/identity.ts index f03aaf8d..451da0ca 100644 --- a/src/slack/identity.ts +++ b/src/slack/identity.ts @@ -5,6 +5,14 @@ export interface ActorAssertion { isExternalGuest?: boolean; isBot?: boolean; displayName?: string; + /** + * Own-team member whose principal could not be resolved in email identity + * mode (email not currently visible in the directory). Refused like a + * guest — fail closed, never a fallback Slack-ID principal — but as its + * OWN state, so logs and the user-facing refusal say "unresolved member" + * instead of masquerading as "external guest" (#626). + */ + principalUnresolved?: true; } export interface SlackUser { @@ -53,14 +61,22 @@ export function classifyUser( ); const displayName = user.profile?.display_name || user.real_name || user.name || user.id || ""; let externalId = String(user.id ?? ""); + // Email mode keys members on their email — including guests, whose + // principal still resolves for refusal copy and audit. An email-LESS + // member with no Slack-side external evidence is own-team but + // principal-unresolved: kept distinct from an external guest (which the + // native flags above decide) so refusal copy and logs name what actually + // happened instead of masquerading as external (#626). + let principalUnresolved = false; if (identity === "email" && !user.is_bot) { const email = (user.profile?.email ?? "").trim().toLowerCase(); if (email.includes("@")) externalId = email; - else isGuest = true; + else if (!isGuest) principalUnresolved = true; } return { externalId, isExternalGuest: isGuest, + ...(principalUnresolved ? { principalUnresolved: true as const } : {}), ...(user.is_bot ? { isBot: true } : {}), ...(displayName ? { displayName } : {}), }; diff --git a/src/slack/turn-handler.ts b/src/slack/turn-handler.ts index 199d7b54..9a4c0edc 100644 --- a/src/slack/turn-handler.ts +++ b/src/slack/turn-handler.ts @@ -296,7 +296,33 @@ export function createTurnHandler(deps: { ...(ids.botHandle ? { botHandle: ids.botHandle } : {}), }; + // A principal-unresolved own-team member (email mode, email not yet + // visible) is refused as its OWN state — fail closed like a guest, but + // named for what it is, both to the sender and in the log, so the + // incident signature is decidable (#626: the refusal used to be + // indistinguishable from external-guest in logs, and its success path + // logged nothing at all). + const unresolvedMember = audience.find((a) => a.principalUnresolved); + if (unresolvedMember) { + console.warn( + `[slack-plugin] refusing turn: own-team member principal unresolved ` + + `(email identity mode; email not visible in the directory) ` + + `user=${unresolvedMember.externalId || "?"} ch=${inc.channel} ts=${inc.ts}`, + ); + if (!inc.unprompted) { + await ephemeralOrSay( + "I can't respond here yet — I couldn't verify your team membership " + + "(your email isn't visible to me). This usually resolves within a " + + "few minutes of the directory refreshing; try again shortly.", + ); + } + return; + } + if (audience.some((a) => a.isExternalGuest) && !(await externalParticipantsEnabled())) { + console.warn( + `[slack-plugin] refusing turn: external guest ch=${inc.channel} ts=${inc.ts}`, + ); if (!inc.unprompted) { await ephemeralOrSay( "I can't respond here — this conversation isn't fully internal. Try a DM or a fully-internal channel.", diff --git a/test/slack-identity.test.ts b/test/slack-identity.test.ts index c260e770..5c4b25ae 100644 --- a/test/slack-identity.test.ts +++ b/test/slack-identity.test.ts @@ -63,7 +63,26 @@ test("classifyUser email mode keys members on their normalized work email", () = test("classifyUser email mode fails closed to guest when a member has no visible email", () => { const a = classifyUser({ id: "U1", team_id: TEAM }, TEAM, "email"); assert.equal(a.externalId, "U1"); - assert.equal(a.isExternalGuest, true); + // Own-team member, principal unresolved: its OWN state, not external + // guest (#626). The refusal is equally closed, but logs and copy say + // what actually happened. + assert.equal(a.isExternalGuest, false); + assert.equal(a.principalUnresolved, true); +}); + +test("classifyUser email mode: native external evidence still wins over unresolved", () => { + // A restricted member with no visible email is an EXTERNAL GUEST, not an + // unresolved own-team member — the third state never masks the flags + // (#626). + const g = classifyUser({ id: "U2", team_id: TEAM, is_restricted: true }, TEAM, "email"); + assert.equal(g.isExternalGuest, true); + assert.equal(g.principalUnresolved, undefined); +}); + +test("classifyUser slack-id mode never marks principalUnresolved", () => { + const a = classifyUser({ id: "U1", team_id: TEAM }, TEAM, "slack-id"); + assert.equal(a.isExternalGuest, false); + assert.equal(a.principalUnresolved, undefined); }); test("classifyUser email mode keeps bots on their Slack id and non-guest", () => {