security: Telegram menu cancel callbacks don't enforce intent ownership #152

Summary

The Telegram menu callback handlers for cancelling intents and cancelling virtual cards trust the intentId payload from the callback without verifying that the intent belongs to the Telegram user who triggered it. A malicious user can intercept their own callback, swap in another user's intentId, and resend — gaining unauthorized read and mutation access.

Surfaced during review of #148 (CodeRabbit comment, PR #148 discussion). Out of scope for #90, hence this dedicated ticket.

Affected paths

In src/telegram/menuHandler.ts, inside handleMenuCallback:

• menu_cancel_confirm:<intentId> → showCancelConfirm(bot, chatId, messageId, payload)
• menu_cancel_do:<intentId> → doCancelIntent(bot, chatId, messageId, payload)
• menu_card_cancel:<intentId> → doCancelCard(bot, chatId, messageId, payload)

These branches run before getUserByChatId(chatId) and never thread userId into the downstream calls. Specifically:

• showCancelConfirm does prisma.purchaseIntent.findUnique({ where: { id: intentId } }) with no userId filter
• doCancelIntent calls expireIntent(intentId) (in src/orchestrator/intentService.ts) — no ownership check
• doCancelCard calls getPaymentProvider().cancelCard(intentId) — no ownership check

Threat model

The Telegram UI only renders these buttons in a user's own menu (showCancelList filters by userId). To exploit, an attacker:

1. Triggers their own legitimate menu and captures the callback_query payload
2. Swaps the intentId portion for a victim's intent ID
3. Resends the modified callback to the bot

Intent IDs are UUIDs (high-entropy, hard to guess), so the practical risk depends on whether IDs leak through other channels (audit exports, logs shared in support, screenshots, future referral flows, etc.). Even without ID leakage this is a defense-in-depth gap and a clear authorization invariant violation.

What an attacker gains

1. Read: another user's intent details (subject, query, status, max budget) via menu_cancel_confirm
2. Mutate: cancel another user's active intent via menu_cancel_do
3. Mutate: cancel another user's virtual card via menu_card_cancel

Acceptance criteria

• Resolve getUserByChatId before dispatching to any of the three branches above (move user lookup above the cancel/card-cancel branches)
• Thread userId into showCancelConfirm, doCancelIntent, doCancelCard
• Constrain DB lookups in showCancelConfirm by { id: intentId, userId } and return the same generic "Intent not found" message on a mismatch (do not leak existence)
• Add a userId (or assertOwnership(intentId, userId)) check at the service-layer entry of expireIntent and the payment provider's cancelCard so any future caller path also gets the check — defense in depth
• Integration tests:
  • cross-user menu_cancel_confirm → "Intent not found"
  • cross-user menu_cancel_do → no state change on victim's intent
  • cross-user menu_card_cancel → no state change on victim's card
• Audit-log every rejected cross-user attempt at warn level with { chatId, telegramUserId, intentId, ownerUserId } for detection

Out of scope (here)

• Rate limiting on rejected attempts (separate ticket if/when needed)
• A broader audit of every other menu_* action — this ticket is scoped to the three cancel paths called out above. A full review of menu callback authorization should happen in a follow-up.

Notes

This bug pre-dates #148 — the dispatch structure for these three branches was unchanged in that PR. Flagged here so it doesn't get lost.