Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
171 changes: 171 additions & 0 deletions bot/src/bridge.teams-persistence.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
import { afterEach, beforeEach, describe, it } from "node:test";
import assert from "node:assert";
import {
recordTeamsConversation,
seedTeamsKnownTeamIds,
clearTeamsKnownTeamIdsForTest,
} from "./bridge.js";

// ── Helpers ──────────────────────────────────────────────────────────────────
// `recordTeamsConversation` fires a write-through POST to the backend whenever
// it observes a new aadGroupId. We don't want test cases to hit a real network
// — so we stub `fetch` with a recorder. Calls are captured for assertion; the
// stub resolves with a synthetic 204 No Content so the helper takes its
// success path.

interface CapturedFetch {
url: string;
init: { method?: string; headers?: Record<string, string>; body?: string };
}

const captured: CapturedFetch[] = [];
let originalFetch: typeof fetch;

function installFetchStub(status: number = 204): void {
originalFetch = globalThis.fetch;
globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => {
captured.push({
url: input.toString(),
init: {
method: init?.method,
headers: init?.headers as Record<string, string> | undefined,
body: init?.body ? String(init.body) : undefined,
},
});
return new Response(null, { status });
}) as typeof fetch;
}

function restoreFetch(): void {
globalThis.fetch = originalFetch;
}

const VALID_AAD_GROUP_ID = "85e9fb0c-6cf9-4e94-9cc4-eb81ea6cd9de";
const ANOTHER_AAD_GROUP_ID = "11111111-2222-3333-4444-555555555555";
const CONN_ID = "test-conn-aabbccdd";
const OTHER_CONN_ID = "other-conn-eeff0011";

const installActivity = (aadGroupId: string, conversationId: string = "19:abcdef@thread.tacv2") => ({
conversation: { id: conversationId, conversationType: "channel" },
channelData: {
team: { id: "team-internal-id", name: "Beever Atlas", aadGroupId },
channel: { id: conversationId, name: "tech-discussion" },
},
serviceUrl: "https://smba.trafficmanager.net/amer/",
});

// ── seedTeamsKnownTeamIds ────────────────────────────────────────────────────

describe("seedTeamsKnownTeamIds", () => {
beforeEach(() => {
clearTeamsKnownTeamIdsForTest();
captured.length = 0;
installFetchStub();
});
afterEach(restoreFetch);

it("hydrates the in-memory Map from a Mongo-supplied list", () => {
seedTeamsKnownTeamIds(CONN_ID, [VALID_AAD_GROUP_ID, ANOTHER_AAD_GROUP_ID]);

// After seeding, a subsequent webhook for the SAME id must NOT fire a
// write-through — the value is already considered known.
recordTeamsConversation(CONN_ID, installActivity(VALID_AAD_GROUP_ID));
assert.strictEqual(
captured.length,
0,
"seeded ids must dedup against incoming webhooks (no POST expected)",
);
});

it("is a no-op for an empty list (preserves legacy cold-start scan path)", () => {
seedTeamsKnownTeamIds(CONN_ID, []);
// A subsequent observation must STILL fire the write-through because the
// empty-seed call did not pre-populate any ids.
recordTeamsConversation(CONN_ID, installActivity(VALID_AAD_GROUP_ID));
assert.strictEqual(captured.length, 1);
});

it("scopes seeded ids per-connection (no cross-connection leak)", () => {
seedTeamsKnownTeamIds(CONN_ID, [VALID_AAD_GROUP_ID]);

// A different connection observing the same team-id must fire its own
// write-through; ids are not shared across connections (Redis cache is
// global but Mongo persistence is per-row).
recordTeamsConversation(OTHER_CONN_ID, installActivity(VALID_AAD_GROUP_ID));
assert.strictEqual(captured.length, 1);
assert.ok(
captured[0].url.includes(encodeURIComponent(OTHER_CONN_ID)),
"write-through URL must target the OTHER connection",
);
});

it("is idempotent across repeated calls (adapter rebuild path)", () => {
seedTeamsKnownTeamIds(CONN_ID, [VALID_AAD_GROUP_ID]);
seedTeamsKnownTeamIds(CONN_ID, [VALID_AAD_GROUP_ID]);
seedTeamsKnownTeamIds(CONN_ID, [VALID_AAD_GROUP_ID, ANOTHER_AAD_GROUP_ID]);

// A webhook for either id is now a no-op write-through.
recordTeamsConversation(CONN_ID, installActivity(VALID_AAD_GROUP_ID));
recordTeamsConversation(CONN_ID, installActivity(ANOTHER_AAD_GROUP_ID));
assert.strictEqual(captured.length, 0);
});
});

// ── Write-through on observed aadGroupId ─────────────────────────────────────

describe("recordTeamsConversation write-through", () => {
beforeEach(() => {
clearTeamsKnownTeamIdsForTest();
captured.length = 0;
installFetchStub();
});
afterEach(restoreFetch);

it("POSTs to the backend the first time a new aadGroupId is observed", () => {
recordTeamsConversation(CONN_ID, installActivity(VALID_AAD_GROUP_ID));

assert.strictEqual(captured.length, 1);
const { url, init } = captured[0];
assert.ok(
url.includes(`/api/internal/connections/${encodeURIComponent(CONN_ID)}/teams-known-team-ids`),
`unexpected URL: ${url}`,
);
assert.strictEqual(init.method, "POST");
assert.deepStrictEqual(JSON.parse(init.body || "{}"), {
aad_group_id: VALID_AAD_GROUP_ID,
});
});

it("dedups subsequent observations of the same id (no second POST)", () => {
recordTeamsConversation(CONN_ID, installActivity(VALID_AAD_GROUP_ID));
recordTeamsConversation(CONN_ID, installActivity(VALID_AAD_GROUP_ID));
recordTeamsConversation(CONN_ID, installActivity(VALID_AAD_GROUP_ID));
assert.strictEqual(captured.length, 1, "duplicate observations must not refire");
});

it("fires a separate POST for a DIFFERENT aadGroupId on the same connection", () => {
recordTeamsConversation(CONN_ID, installActivity(VALID_AAD_GROUP_ID));
recordTeamsConversation(CONN_ID, installActivity(ANOTHER_AAD_GROUP_ID));
assert.strictEqual(captured.length, 2);
const ids = captured.map((c) => JSON.parse(c.init.body || "{}").aad_group_id).sort();
assert.deepStrictEqual(ids, [ANOTHER_AAD_GROUP_ID, VALID_AAD_GROUP_ID].sort());
});

it("rejects malformed aadGroupId without firing a POST", () => {
const activity = installActivity("not-a-guid-at-all");
recordTeamsConversation(CONN_ID, activity);
assert.strictEqual(captured.length, 0, "non-GUID aadGroupId must be filtered client-side");
});

it("does not fire when aadGroupId is absent (regular channel-message activity)", () => {
const activity = {
conversation: { id: "19:abcdef@thread.tacv2", conversationType: "channel" },
channelData: {
team: { id: "team-internal-id", name: "Beever Atlas" }, // no aadGroupId
channel: { id: "19:abcdef@thread.tacv2", name: "tech-discussion" },
},
};
recordTeamsConversation(CONN_ID, activity);
assert.strictEqual(captured.length, 0);
});
});
153 changes: 144 additions & 9 deletions bot/src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1249,24 +1249,135 @@

/**
* Per-connection set of AAD group IDs (a.k.a. "team ids" in Graph) for teams
* the bot is installed in. Populated by `recordTeamsConversation` when a Bot
* Framework activity carries `channelData.team.aadGroupId` (present in install
* and conversationUpdate events). Used by `TeamsBridge.listChannels` to call
* `GET /teams/{aadGroupId}/channels` without any prior @mention.
* the bot is installed in. Populated from three sources, in priority order:
*
* Survives adapter recycles (module-level singleton) but is wiped on process
* restart. `listChannels` performs a one-shot Redis SCAN per connection to
* re-seed it on first cold-start (see `teamsColdStartScanned`).
* 1. Mongo (via `seedTeamsKnownTeamIds`): hydrated at startup by the bot's
* connection loader from the persistent `teams_known_team_ids` field on
* each Teams `PlatformConnection`. This is the parity path with how
* Slack/Discord/Mattermost bootstrap from their bot tokens in Mongo —
* identity survives Redis loss AND bot restart with zero webhooks.
* 2. Bot Framework activities (via `recordTeamsConversation`): every install
* and `conversationUpdate` carries `channelData.team.aadGroupId`. New
* values are written through to Mongo (fire-and-forget) so future
* restarts hydrate path 1 from a complete set.
* 3. Redis cold-start SCAN (via `TeamsBridge.resolveTeamIds`): legacy
* fallback for connections that pre-date the Mongo persistence field.
* Retired automatically once a connection has at least one observed
* team (see `seedTeamsKnownTeamIds` flipping `teamsColdStartScanned`).
*/
const teamsKnownTeamIds = new Map<string, Set<string>>();

/**
* H3/M1: one-shot guard — tracks which connectionIds have already completed
* the cold-start Redis SCAN. Prevents re-scanning on every `listChannels` call
* and avoids the blocking `KEYS` pattern entirely.
* and avoids the blocking `KEYS` pattern entirely. Also flipped by
* `seedTeamsKnownTeamIds` so a Mongo-hydrated connection never falls back to
* the Redis scan on its first call.
*/
const teamsColdStartScanned = new Set<string>();

/** Seed `teamsKnownTeamIds` for a connection from the durable Mongo record.
* Called by the bot's startup loader (one call per Teams connection) and
* again on each adapter rebuild — idempotent. A no-op when the list is
* empty so legacy connections without persisted team-ids still benefit
* from the Redis cold-start scan. */
export function seedTeamsKnownTeamIds(connectionId: string, teamIds: string[]): void {
if (!connectionId || !teamIds || teamIds.length === 0) return;
let teamSet = teamsKnownTeamIds.get(connectionId);
if (!teamSet) {
teamSet = new Set();
teamsKnownTeamIds.set(connectionId, teamSet);
}
for (const id of teamIds) {
if (id) teamSet.add(id);
}
// Mongo is now authoritative for this connection; suppress the Redis scan
// path entirely so a stale/wiped cache can't race the hydrated state.
teamsColdStartScanned.add(connectionId);
}

/** Test-only: drop cached state for a connection (or all). Clears the
* in-memory team-id Map, the cold-start scan guard, AND the backend
* write-through dedup Set so tests don't bleed across cases. Production
* code prunes the registry via `pruneStaleTeamsConversations`. */
export function clearTeamsKnownTeamIdsForTest(connectionId?: string): void {
if (connectionId) {
teamsKnownTeamIds.delete(connectionId);
teamsColdStartScanned.delete(connectionId);
// Drop write-through dedup entries scoped to this connection so the
// next observation re-fires the POST. Keyed `${connId}:${aadId}`.
for (const key of teamsWriteThroughInFlight) {
if (key.startsWith(`${connectionId}:`)) {
teamsWriteThroughInFlight.delete(key);
}
}
} else {
teamsKnownTeamIds.clear();
teamsColdStartScanned.clear();
teamsWriteThroughInFlight.clear();
}
}

/** Backend write-through dedup: keyed by `${connectionId}:${aadGroupId}` so a
* burst of webhooks for the same team doesn't fire N concurrent POSTs. On
* success the entry stays in place for the process lifetime (Mongo already
* has the value). On transient failure (5xx/network) we clear the entry so
* the next observation re-attempts. */
const teamsWriteThroughInFlight = new Set<string>();

/** Fire-and-forget POST that persists an observed AAD group id to Mongo so
* it survives bot restart AND a Redis cache wipe. Called from
* `recordTeamsConversation` whenever a NEW team-id surfaces for a
* connection. Errors are swallowed at the call site — the in-memory Map
* is the source of truth for the live process; the POST just hydrates
* Mongo for next startup. */
async function persistTeamsKnownTeamIdToBackend(
connectionId: string,
aadGroupId: string,
): Promise<void> {
const dedupKey = `${connectionId}:${aadGroupId}`;
if (teamsWriteThroughInFlight.has(dedupKey)) return;
teamsWriteThroughInFlight.add(dedupKey);

const backendUrl = process.env.BACKEND_URL || "http://localhost:8000";
const bridgeKey = process.env.BRIDGE_API_KEY || "";
const headers: Record<string, string> = { "Content-Type": "application/json" };
if (bridgeKey) headers["Authorization"] = `Bearer ${bridgeKey}`;

try {
const resp = await fetch(
`${backendUrl}/api/internal/connections/${encodeURIComponent(connectionId)}/teams-known-team-ids`,
{
method: "POST",
headers,
body: JSON.stringify({ aad_group_id: aadGroupId }),
signal: AbortSignal.timeout(5000),
},
);
if (resp.ok) return;
// 4xx is terminal (404 unknown connection, 422 wrong platform, 400 bad
// GUID). Leave the dedup entry in place so we don't loop on the same
// bad value.
if (resp.status >= 400 && resp.status < 500) {
console.warn(
`TeamsBridge: backend rejected aadGroupId persist (${resp.status}) for connection ${connectionId}`,
);
return;
}
// 5xx → transient; allow retry on next observation.
console.warn(
`TeamsBridge: backend returned ${resp.status} persisting aadGroupId for ${connectionId}; will retry`,
);
teamsWriteThroughInFlight.delete(dedupKey);
} catch (err) {
console.warn(
`TeamsBridge: failed to persist aadGroupId for ${connectionId}:`,

Check failure

Code scanning / CodeQL

Use of externally-controlled format string High

Format string depends on a
user-provided value
.
Format string depends on a
user-provided value
.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
safeErrorMessage(err),
);
teamsWriteThroughInFlight.delete(dedupKey);
}
}

/**
* A Microsoft Graph team-id is the team's AAD group object id — a GUID. Used to
* validate teamIds sourced from the shared Redis channelContext cache before
Expand Down Expand Up @@ -1377,14 +1488,28 @@
// TeamsBridge.listChannels can enumerate channels via Graph without any
// prior @mention. Bot Framework install/conversationUpdate activities carry
// `channelData.team.aadGroupId`; regular channel-message activities may not.
//
// Two-layer persistence:
// • In-memory Map — covers the live process; lost on bot restart.
// • Mongo via fire-and-forget POST — survives bot restart AND a Redis
// cache wipe. Only fires when the id is NEW for this connection so
// a steady stream of channel-message webhooks doesn't hammer the
// backend with no-op upserts. Backend dedups via `$addToSet`.
const aadGroupId = activity.channelData?.team?.aadGroupId;
if (aadGroupId) {
if (aadGroupId && TEAMS_AAD_GROUP_ID_RE.test(aadGroupId)) {
let teamSet = teamsKnownTeamIds.get(connectionId);
if (!teamSet) {
teamSet = new Set();
teamsKnownTeamIds.set(connectionId, teamSet);
}
const isNew = !teamSet.has(aadGroupId);
teamSet.add(aadGroupId);
if (isNew) {
// Fire-and-forget — never block webhook processing on the backend
// round-trip. The helper handles its own errors + dedup so an
// unhandled rejection can't escape this scope.
void persistTeamsKnownTeamIdToBackend(connectionId, aadGroupId);
}
}
}

Expand Down Expand Up @@ -1538,7 +1663,17 @@
teamSet = new Set();
teamsKnownTeamIds.set(this.connectionId, teamSet);
}
const wasNew = !teamSet.has(tId);
teamSet.add(tId);
// Persist any id we recover from the Redis-cache cold-start
// path too — otherwise an EXISTING connection that was
// already populated via a prior webhook never seeds Mongo
// (the in-memory dedup in `recordTeamsConversation` would
// suppress the write-through forever). Fire-and-forget;
// backend `$addToSet` keeps it idempotent.
if (wasNew) {
void persistTeamsKnownTeamIdToBackend(this.connectionId, tId);
}
}
}
} catch {
Expand Down
Loading
Loading