[Detail Bug] Connection quota: Concurrent Telegram bot connects can exceed plan connection slot limit

[detail-app]

Detail Bug Report

https://app.detail.dev/org_ee14aa77-b24a-40b2-b22d-66bd31931f4a/bugs/bug_943b2a74-99ff-4e3f-9121-1def68ccf391

Introduced in #332 by @setkyar on Sep 9, 2026

Summary

• Context: The connection-quota system (introduced across apps/api/src/services/whatsapp/connection.ts and apps/api/src/services/connection-quota.service.ts) caps how many paid "connection slots" a workspace may use across all channels (WhatsApp linked devices + Telegram bots), enforced at connect time.
• Bug: The Telegram bot creation route performs the quota check outside its insert transaction and acquires no advisory lock, so the check-then-insert sequence is not atomic; the whatsapp spawnConnection path serializes the same sequence with pg_advisory_xact_lock(hashtextextended(${companyId}, 0)), but the Telegram path never acquires that lock.
• Actual vs. expected: Two concurrent POST /channel-accounts/telegram-bot requests (or a concurrent Telegram connect + WhatsApp spawn) both observe the same pre-insert slot count, both pass the used < max gate, and both insert, leaving the workspace with more active connections than its plan allows.
• Impact: A workspace can exceed its paid connection limit, defeating the entitlement enforcement commit afe09e5 introduced. The over-limit channel_accounts rows persist and count toward countUsedConnectionSlots regardless of whether webhook configuration succeeds: the insert transaction (channel-accounts.ts:234-318) commits the row with status: "connecting" before configureTelegramWebhook is called at channel-accounts.ts:322. On webhook failure the row is set to status: "error" (line 328) and a 502 is returned, but countUsedConnectionSlots counts any row where status != "archived" (connection-quota.service.ts:34) — so a failed account occupies a slot just as a connected one does, and persists until an operator manually archives it.

Code with Bug

apps/api/src/routes/channel-accounts.ts:

    // The plan sells connection slots, not WhatsApp slots. Reusing the same
    // ceiling here keeps a workspace from adding channel accounts outside the
    // plan it pays for. Reconnecting an account that already exists does not
    // consume a new slot, so only a genuinely new account is counted.
    if (!existingAccount) {
      const maxConnections = await getMaxConnections(companyId);
      const used = await countUsedConnectionSlots(tenantDb);  // <-- BUG 🔴 reads the slot count outside the insert transaction, with no advisory lock
      if (used >= maxConnections) {
        return c.json(
          {
            error: "Connection limit reached for this plan",
            code: "MAX_CONNECTIONS_EXCEEDED",
            used,
            max: maxConnections,
          },
          402,
        );
      }
    }

    // ... identity lookup, schema lookup ...

    await db.transaction().execute(async (trx) => {
      const tenant = trx.withSchema(
        company.schema_name,
      ) as unknown as Transaction<TenantDatabase>;
      if (existingAccount) {
        // ... update existing ...
      } else {
        await tenant
          .insertInto("channel_accounts")  // <-- BUG 🔴 insert never re-checks quota and never acquires the company-scoped advisory lock
          .values({ /* ... */ })
          .execute();
      }
      // ... store credentials, insert ingress route ...
    });

Explanation

• This is a TOCTOU race: quota is checked on tenantDb before starting the insert transaction, and the insert transaction neither re-checks the quota nor serializes with other connect operations.
• With PostgreSQL default READ COMMITTED, two concurrent requests can both read the same used value (e.g., 4 of 5), both pass used < maxConnections, and both commit inserts, leaving the tenant above the cap.
• Cross-channel: WhatsApp spawnConnection uses a company-scoped advisory lock, but advisory locks only block other sessions that acquire the same lock; since the Telegram route never acquires the lock, it can interleave with WhatsApp’s transaction and still over-insert.
• countUsedConnectionSlots counts rows where status != "archived", so even Telegram bot rows committed with status: "error" after webhook failure still consume a slot until manually archived.

Codebase Inconsistency

The WhatsApp path correctly serializes the shared count+insert sequence using pg_advisory_xact_lock(hashtextextended(${companyId}, 0)) inside the transaction, but the Telegram bot creation path does not.

Recommended Fix

Move the quota check inside the Telegram insert transaction and acquire the same company-scoped advisory lock before counting, mirroring spawnConnection. Preserve the Telegram route’s existing 402 response contract (it currently returns a 402 JSON body with { error, code: "MAX_CONNECTIONS_EXCEEDED", used, max }; throwing MaxConnectionsExceededError directly would instead flow through app.onError as a 429 with a generic message unless explicitly caught and translated).

History

This bug was introduced in commit afe09e5. The commit unified WhatsApp and Telegram under a single countUsedConnectionSlots quota counter; on the WhatsApp path it swapped the old WhatsApp-only count for the shared count inside an existing advisory-lock-protected transaction (remaining race-free), but on the Telegram path it added the same count as a bare check outside the insert transaction with no advisory lock — creating the TOCTOU the moment the unified quota enforcement came into existence.