Skip to content

fix(security): add authorization and input validation for menu and preferences#95

Open
JonasBaeumer wants to merge 3 commits into
mainfrom
feat/issue-21-preferences
Open

fix(security): add authorization and input validation for menu and preferences#95
JonasBaeumer wants to merge 3 commits into
mainfrom
feat/issue-21-preferences

Conversation

@JonasBaeumer

Copy link
Copy Markdown
Owner

Closes #91

Summary

  • Intent ownership checks: menu_cancel_confirm, menu_cancel_do, and menu_card_cancel now run after the user lookup and verify intent.userId === user.id before acting. Forged or replayed callback payloads from other users get an error menu instead.
  • Enum validation: menu_pref_policy validates the payload against Object.values(CardCancelPolicy) before casting, preventing invalid values from being written to the DB.
  • Auth on preferences endpoint: PATCH /v1/users/:userId/preferences now requires a valid Bearer token (userAuthMiddleware) and a matching userId. Returns 401 without auth, 403 for a different user. Writes a PREFERENCES_UPDATED audit event on success.
  • Null guard in callbackHandler: Replaced chatId!/messageId! non-null asserts with an explicit if (chatId === undefined || messageId === undefined) return; guard to prevent silent runtime crashes on inline keyboard callbacks.

Test plan

  • All unit tests pass: npm test -- --testPathPattern=tests/unit
  • New unit tests cover ownership failure (permission error message shown, action not taken) for all three cancel actions
  • New unit test covers invalid CardCancelPolicy payload → error shown, no DB write
  • Integration tests for preferences updated: now include auth setup, 401/403 coverage, and audit event assertion
  • Integration tests require DB: docker compose up -d && npm run test:integration -- --testPathPattern=telegram/menuHandler

…eferences

Closes #91

- Verify intent ownership before cancel/card operations in Telegram menu
  (menu_cancel_confirm, menu_cancel_do, menu_card_cancel now require the
  intent's userId to match the requesting user's id)
- Validate CardCancelPolicy enum value before casting from raw callback payload
- Add userAuthMiddleware + ownership check to PATCH /v1/users/:userId/preferences,
  and write a PREFERENCES_UPDATED audit event on success
- Guard undefined chatId/messageId in callbackHandler before menu dispatch
  instead of using non-null assertions
- Add promptMessageId to PrefSession interface
- Use CardCancelPolicy enum instead of string literals
- Move CancelCardJob to contracts and add QUEUE_NAMES constants
- Replace duplicate ACTIVE_INTENT_STATUSES arrays with ACTIVE_STATES
- Tighten ReconciliationReport types (PotStatus, cardStatus union)
@JonasBaeumer JonasBaeumer force-pushed the feat/issue-21-preferences branch from 52b5976 to fb2aeec Compare March 29, 2026 15:46
@JonasBaeumer JonasBaeumer assigned JonasBaeumer and unassigned Hajuj Mar 29, 2026
@JonasBaeumer JonasBaeumer requested a review from Hajuj March 29, 2026 15:48
refactor(contracts): type system and contract cleanup (#92)

@georgyia georgyia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very close to merge — the security intent/ownership + validation changes look correct.

Small blockers to address before merge:

  • tests/integration/telegram/menuHandler.test.ts has 1 failing assertion: it expects a generic “went wrong” string, but the handler now returns the specific ⚠️ Intent not found. message. Please update the expectation so the suite is green.
  • The same integration suite is gated on STRIPE_SECRET_KEY even though it mocks Telegram/BullMQ and only needs Postgres+Redis. Removing that gating will ensure these security tests actually run in CI/dev environments.

Once those two are fixed, I’m happy to approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: missing authorization and input validation in Telegram menu and preferences API

3 participants