fix(telegram,encryption): restore DEK-based decryption - #548
Conversation
- Add TelegramConnect entity, DTOs, and migration for pairing tokens - Implement telegram-connect, telegram-polling, telegram-webhook services - Add /integrations/:id/connects endpoints for token generation and status - Add /integrations/:id/webhook/:integrationId endpoint for Telegram updates - Create console Telegram connect UI with QR code display and polling - Add CopyableValue component for token display - Update Integration entity with telegramConnects relation - Update generated API hooks (orval)
- console/package.json: remove duplicate qrcode.react entry - console/copyable-value.tsx: wrap clipboard write in try/catch - console/telegram-connect.tsx: add !isError to empty state, fix deepLink to use ?start= - core-api/telegram-connect.entity.ts: type-only import for Relation - core-api/telegram-connect.service.ts: fix HTML markup (*bold* -> <b>bold</b>), add userId scope to disconnect(), add AbortSignal.timeout to fetch calls - core-api/telegram-webhook.service.ts: stop logging token prefix - core-api/telegram.connector.ts: add AbortSignal.timeout, continue on per-chat failure instead of throwing on first error - core-api/integrations.controller.ts: forward userId to disconnect
- Refactor encryption.util.ts: ENCRYPTION_KEYS env var, index-prefix ciphertext - Add parseEncryptionKeys() and getActiveEncryptionKey() - Create workspace-encryption.util.ts (generateDEK, wrapDEK, unwrapDEK, encryptWithDEK, decryptWithDEK) - Add comprehensive test suite (12 tests, all passing)
- Add dek (encrypted DEK) and dekAt columns to Workspace entity - Create migration 1785000000000-AddWorkspaceDEK
- Add WorkspaceEncryptionService for DEK lifecycle (generate, wrap, unwrap) - Encrypt sensitive fields in agents, assets, integrations, notifications with DEK - Add dek/dekAt columns and migration for existing workspaces - Backfill missing DEKs on startup via distributed lock - Replace dynamic require() with static import in workspace-encryption.util - Fix lint: remove unused decrypt import, unnecessary type assertions - Fix test specs: add WorkspaceEncryptionService mock to assets and workspaces - Remove stale migration file 1776960340581-migrations.ts
…envelope-encryption-with-key-rotation
- Remove encryptCache/decryptCache (breaks AES-CBC randomized IV, leaks memory) - Fix evictOldestIfNeeded generic cast to use IteratorResult.done check - Use conditional update WHERE dek IS NULL in ensureDEK to prevent race - Move DEK/decrypt inside try/catch in telegram pollBot for proper error logging - Move decrypt inside Promise.allSettled callback in notifications processor
… feat/workspace-level-envelope-encryption-with-key-rotation
📝 WalkthroughWalkthroughThe changes update workspace DEK provisioning and caching, move notification integration filtering into concurrent dispatch, and broaden Telegram polling error handling to cover setup failures while treating HTTP 409 responses without warning logs. ChangesCore service behavior updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core-api/src/services/workspace-encryption/workspace-encryption.service.ts`:
- Around line 35-42: Replace evictOldestIfNeeded with a synchronous setDekCache
helper that checks whether workspaceId already exists before evicting, then
inserts or updates the DEK without reducing capacity during concurrent requests.
Update ensureDEK and getDEK to use setDekCache for all cache writes, removing
their inline eviction and set operations while preserving existing cache
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 55cea6b1-2e5a-45f2-aa17-73c9f563994c
📒 Files selected for processing (3)
core-api/src/modules/integrations/telegram-polling.service.tscore-api/src/modules/notifications/processors/notifications.processor.tscore-api/src/services/workspace-encryption/workspace-encryption.service.ts
| private evictOldestIfNeeded(): void { | ||
| if (this.dekCache.size >= WorkspaceEncryptionService.MAX_CACHE_SIZE) { | ||
| const oldest = this.dekCache.keys().next(); | ||
| if (!oldest.done) { | ||
| this.dekCache.delete(oldest.value); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Prevent cache capacity degradation during concurrent requests.
When multiple concurrent requests attempt to fetch and cache a missing DEK for the same workspaceId (e.g., during a spike in webhook events for a legacy workspace), evictOldestIfNeeded() may execute multiple times. Because updating an existing key in a Map does not increase its size, the cache's total capacity will permanently shrink by 1 for each concurrent identical request when the cache is at its max size.
To fix this, check if the key already exists before evicting. The safest approach is to consolidate the eviction and insertion logic into a single synchronous helper method to eliminate the race condition.
🛠️ Proposed helper to replace evictOldestIfNeeded
- private evictOldestIfNeeded(): void {
- if (this.dekCache.size >= WorkspaceEncryptionService.MAX_CACHE_SIZE) {
- const oldest = this.dekCache.keys().next();
- if (!oldest.done) {
- this.dekCache.delete(oldest.value);
- }
- }
- }
+ private setDekCache(workspaceId: string, dek: Buffer): void {
+ if (!this.dekCache.has(workspaceId) && this.dekCache.size >= WorkspaceEncryptionService.MAX_CACHE_SIZE) {
+ const oldest = this.dekCache.keys().next();
+ if (!oldest.done) {
+ this.dekCache.delete(oldest.value);
+ }
+ }
+ this.dekCache.set(workspaceId, dek);
+ }After adding this helper, replace the inline set operations in ensureDEK (lines 111-112, 121-122) and getDEK (lines 151-152) with:
this.setDekCache(workspaceId, dek);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core-api/src/services/workspace-encryption/workspace-encryption.service.ts`
around lines 35 - 42, Replace evictOldestIfNeeded with a synchronous setDekCache
helper that checks whether workspaceId already exists before evicting, then
inserts or updates the DEK without reducing capacity during concurrent requests.
Update ensureDEK and getDEK to use setDekCache for all cache writes, removing
their inline eviction and set operations while preserving existing cache
behavior.
Summary by CodeRabbit