Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/slack/identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 } : {}),
};
Expand Down
26 changes: 26 additions & 0 deletions src/slack/turn-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
21 changes: 20 additions & 1 deletion test/slack-identity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down