feat(secure): level envelope encryption with key rotation - #546
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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesWorkspace envelope encryption is added with rotating KEKs, per-workspace DEKs, migration and backfill handling, and DEK-aware LLM and integration flows. A local Telegram polling service processes updates when webhooks are unavailable. API key revocation query behavior is adjusted. Workspace encryption
Telegram polling
API key update behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TelegramPollingService
participant RedisLockService
participant Telegram
participant TelegramWebhookService
TelegramPollingService->>RedisLockService: acquire per-bot lock
TelegramPollingService->>Telegram: getUpdates(offset, timeout)
TelegramPollingService->>TelegramWebhookService: processUpdate(update)
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 |
…envelope-encryption-with-key-rotation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (15)
console/src/pages/settings/components/api-keys-settings.tsx (1)
76-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider supporting inline actions in
CopyableValue.By placing
CopyableValueabove a separatedivfor theRotatebutton, the "Copy" and "Rotate" buttons will now render on separate rows (sinceCopyableValueprovides its own centered flex container for the copy button).If you want to maintain a side-by-side button layout, you could optionally update
CopyableValueto accept anactionsorchildrenprop and render it alongside the copy button:// In CopyableValue: <div className="flex justify-center gap-2"> <Button ...>Copy</Button> {actions} </div> // In ApiKeysSettings: <CopyableValue value={...} disabled={...} actions={ <Button variant="secondary" size="sm" onClick={handleRotate} disabled={...}> <RefreshCw className={`h-4 w-4 ${isRotating ? 'animate-spin' : ''}`} /> {isRotating ? 'Rotating...' : 'Rotate'} </Button> } />🤖 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 `@console/src/pages/settings/components/api-keys-settings.tsx` around lines 76 - 82, Update CopyableValue to accept an optional actions or children prop and render it beside the copy button within its existing centered flex container. Move the Rotate button from the separate layout in ApiKeysSettings into this prop, preserving its current handler, disabled state, icon, and rotating label so Copy and Rotate remain side by side.core-api/src/modules/integrations/validators/integration.validator.ts (1)
139-154: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent catch masks genuine decryption failures, not just legacy plaintext.
decryptWithDEKalready tries DEK → indexed KEK → trial-all-KEK before throwing. A throw here means every decryption path failed — likely signaling a real problem (stale/incorrect DEK, KEK misconfiguration) rather than "value was never encrypted." Currently this is swallowed silently and the raw ciphertext is left in place as if it were the plaintext value, which can cause confusing downstream auth failures (e.g., wrong Telegram/API credentials sent to providers) with no diagnostic trail.Consider logging a warning when the catch is hit so failures are observable, while keeping the same fallback behavior.
🤖 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/modules/integrations/validators/integration.validator.ts` around lines 139 - 154, Update the catch block in decryptSensitiveConfigFields to log a warning when decryptWithDEK fails, including sufficient context to diagnose the failed sensitive field. Preserve the existing fallback behavior by leaving the original value unchanged after the warning.core-api/src/common/utils/workspace-encryption.util.ts (1)
32-39: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMinor:
parseEncryptionKeys()is computed twice perwrapDEKcall.
getActiveEncryptionKey()(line 34) internally callsparseEncryptionKeys(), and line 36 calls it again just to getactiveIndex. Derive both from a single call to avoid the duplicate split+SHA-256 work.♻️ Proposed refactor
export function wrapDEK(dek: Buffer): string { const iv = randomBytes(IV_LENGTH); - const cipher = createCipheriv(ALGORITHM, getActiveEncryptionKey(), iv); + const allKeys = parseEncryptionKeys(); + const activeIndex = allKeys.length - 1; + const cipher = createCipheriv(ALGORITHM, allKeys[activeIndex], iv); const encrypted = Buffer.concat([cipher.update(dek), cipher.final()]); - const allKeys = parseEncryptionKeys(); - const activeIndex = allKeys.length - 1; return `${activeIndex}:${iv.toString('hex')}:${encrypted.toString('hex')}`; }🤖 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/common/utils/workspace-encryption.util.ts` around lines 32 - 39, Update wrapDEK to call parseEncryptionKeys only once, derive the active encryption key and activeIndex from that shared result, and use both when creating the cipher and formatting the return value. Avoid using getActiveEncryptionKey if it would trigger a second parse.core-api/src/common/utils/encryption.util.ts (2)
64-104: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winIndex-prefixed "O(1) lookup" isn't actually used for key selection — falls back to O(n) trial-decryption every time. Both
decrypt()andunwrapDEK()parse and validate thekeyIndexprefix but then discard it, settingkeys = allKeysand looping over every configured KEK regardless. The docstrings in both files promise O(1) lookup by index, but the code never indexes intoallKeysdirectly.
core-api/src/common/utils/encryption.util.ts#L64-L104: after validatingkeyIndex < allKeys.length, useallKeys[keyIndex]as the primary (or sole) decryption key instead of assigningkeys = allKeys.core-api/src/common/utils/workspace-encryption.util.ts#L46-L82: apply the same fix — selectallKeys[keyIndex]directly for the indexed-prefix branch instead ofkeys = allKeys.🤖 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/common/utils/encryption.util.ts` around lines 64 - 104, Update decrypt() in core-api/src/common/utils/encryption.util.ts at lines 64-104 to use allKeys[keyIndex] directly after validating the index, while preserving all-key fallback behavior for the legacy unprefixed format. Apply the same indexed-key selection in unwrapDEK() in core-api/src/common/utils/workspace-encryption.util.ts at lines 46-82; both indexed branches must avoid trial-decrypting every configured key.
92-104: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftUnauthenticated CBC + multi-key trial-decryption widens false-decrypt risk.
decrypt()tries every configured KEK in a loop and returns on the firstdecipher.final()that doesn't throw. Since AES-256-CBC has no built-in authentication (no GCM tag/HMAC), a wrong key can occasionally produce syntactically-valid PKCS7 padding and silently return garbage plaintext instead of throwing — and trying more keys during rotation increases that odds. This risk compounds with the fact that the key-index prefix isn't actually used to pick a single candidate key (see below).Recommend migrating to an AEAD mode (AES-256-GCM) for new ciphertext, or at minimum adding an HMAC (encrypt-then-MAC) over CBC ciphertexts so tampered/mis-keyed decrypts fail deterministically rather than silently succeeding with wrong plaintext.
🤖 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/common/utils/encryption.util.ts` around lines 92 - 104, Update the encryption/decryption implementation around decrypt() to use authenticated encryption for new ciphertext, preferably AES-256-GCM with its authentication tag verified before returning plaintext. If CBC compatibility must remain, add an encrypt-then-MAC covering the ciphertext and verify it before decryption, while preserving an explicit legacy migration path as needed; do not rely on decipher.final() or multi-key trial success as authentication.core-api/src/common/utils/workspace-encryption.util.spec.ts (1)
35-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test coverage for actual multi-key KEK rotation.
All tests run against whatever single default key
parseEncryptionKeys()resolves to. Since key rotation (adding a new KEK while old KEKs remain decrypt-only) is this PR's core feature, consider adding a test that setsprocess.env.ENCRYPTION_KEYSto multiple comma-separated keys, wraps a DEK, then appends another key and confirms the original wrapped DEK still unwraps via its index prefix.🤖 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/common/utils/workspace-encryption.util.spec.ts` around lines 35 - 59, Extend the “wrapDEK / unwrapDEK” tests to exercise multi-key rotation: configure process.env.ENCRYPTION_KEYS with multiple comma-separated keys, wrap a DEK using the initial key set, append a new key, and verify the original wrapped value still unwraps to the same DEK through its stored key index. Restore the environment variable after the test to avoid affecting other cases.core-api/src/modules/workspaces/entities/workspace.entity.ts (1)
126-128: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd type validation for
dekAt.For consistency with other fields in this entity that utilize
class-validator(such asdekusing@IsString()), consider adding the@IsDate()decorator to validate the type ofdekAt.🛠️ Proposed fix
`@IsOptional`() + `@IsDate`() `@Column`({ type: 'timestamp', nullable: true }) dekAt?: Date | null;🤖 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/modules/workspaces/entities/workspace.entity.ts` around lines 126 - 128, Update the dekAt property in the workspace entity to add class-validator date validation with `@IsDate`(), while preserving its existing optional and nullable behavior.core-api/src/modules/workspaces/workspaces.service.spec.ts (1)
165-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the workspace DEK creation contract.
The empty mock only satisfies dependency injection. Add
generateWrappedDEKand test thatcreateWorkspacepersists bothdekanddekAt.Proposed mock
- const mockWorkspaceEncryptionService = {}; + const mockWorkspaceEncryptionService = { + generateWrappedDEK: jest.fn().mockReturnValue('wrapped-dek'), + };As per coding guidelines, “Mock all external dependencies in core-api Jest tests.”
Also applies to: 198-201
🤖 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/modules/workspaces/workspaces.service.spec.ts` around lines 165 - 166, The empty mockWorkspaceEncryptionService does not cover the workspace DEK creation contract. Add a generateWrappedDEK mock to mockWorkspaceEncryptionService, then update the createWorkspace tests to verify the persisted workspace includes both dek and dekAt values returned by the mock; keep all external dependencies mocked.Source: Coding guidelines
core-api/src/modules/integrations/telegram-connect.service.ts (2)
40-46: 📐 Maintainability & Code Quality | 🔵 TrivialStatic-state wiring for
TelegramConnectorcouples lifecycle to module init order.
TelegramConnector.setConnectRepomutates a static class property instead of using Nest DI, sopush()depends ononModuleInithaving already run. It works given Nest's lifecycle ordering, but it's untestable in isolation (global mutable state) and fragile if the connector is ever resolved outside this module's bootstrap.🤖 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/modules/integrations/telegram-connect.service.ts` around lines 40 - 46, Remove the static repository wiring from onModuleInit and refactor TelegramConnector to receive the connect repository through Nest dependency injection, storing it as instance state used by push(). Update its provider/module registration and constructor dependencies so direct test instantiation and resolution outside module bootstrap do not depend on global mutable state.
356-370: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo shared Telegram Bot API client — chat-ID lookup and message-sending are each implemented twice. Both files independently query connected
telegramChatIds and independently POST to the Bot API'ssendMessage, with nearly identical URL construction, timeout, and error handling.
core-api/src/modules/integrations/telegram-connect.service.ts#L356-L370:getConnectedChatIdsduplicates the query now inlined inTelegramConnector.push; extract a shared query helper (or verify/remove this method if it's now unused).core-api/src/modules/integrations/telegram-connect.service.ts#L375-L403:sendTelegramMessageduplicates the per-chat send call inTelegramConnector.push; extract a shared "send Telegram message" helper used by both.core-api/src/modules/integrations/connectors/telegram.connector.ts#L98-L117: replace the inline connected-chats query with the shared helper (or a call intoTelegramConnectService.getConnectedChatIds).core-api/src/modules/integrations/connectors/telegram.connector.ts#L119-L164: replace the inline send loop with the shared "send Telegram message" helper (and parallelize withPromise.allSettled, per the performance note on this range).🤖 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/modules/integrations/telegram-connect.service.ts` around lines 356 - 370, Remove the duplicated Telegram API logic by making TelegramConnector.push use shared helpers from TelegramConnectService. In core-api/src/modules/integrations/telegram-connect.service.ts lines 356-370, retain or extract getConnectedChatIds for the connected-chat query; in lines 375-403, extract or reuse sendTelegramMessage for the Bot API request. In core-api/src/modules/integrations/connectors/telegram.connector.ts lines 98-117 and 119-164, replace the inline query and send loop with those helpers, and dispatch sends via Promise.allSettled while preserving existing timeout and error-handling behavior.core-api/src/modules/integrations/telegram-webhook.service.ts (1)
45-78: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
processUpdatenever threads anintegrationId, so neither call site can scope/verify it. The webhook route's:integrationIdpath segment and the poller'sintegration.idare both available but unused, so a validconnectTokenis accepted regardless of which integration's URL/bot delivered it.
core-api/src/modules/integrations/telegram-webhook.service.ts#L45-L78: changeprocessUpdate(update)toprocessUpdate(integrationId, update)and checkconnect.integrationId === integrationIdbefore callingconfirmConnection.core-api/src/modules/integrations/integrations.controller.ts#L213-L220: pass the already-extractedintegrationIdroute param intoprocessUpdate.core-api/src/modules/integrations/telegram-polling.service.ts#L139-L147: passintegration.idintoprocessUpdateat this call site.🤖 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/modules/integrations/telegram-webhook.service.ts` around lines 45 - 78, Thread the integration scope through Telegram update processing: update TelegramWebhookService.processUpdate to accept integrationId, verify the resolved connection’s integrationId matches it before calling confirmConnection, and reject mismatches. In core-api/src/modules/integrations/integrations.controller.ts lines 213-220, pass the extracted route integrationId; in core-api/src/modules/integrations/telegram-polling.service.ts lines 139-147, pass integration.id.core-api/src/modules/integrations/integrations.module.ts (1)
4-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
@/alias for imports in core-api.As per coding guidelines, imports within
core-api/srcmust use the@/alias instead of relative paths.
core-api/src/modules/integrations/integrations.module.ts#L4-L10: Refactor these relative imports to use the@/alias (e.g.,@/modules/integrations/entities/integration.entity).core-api/src/modules/integrations/entities/integration.entity.ts#L4-L4: Refactor this relative import to use@/modules/integrations/entities/telegram-connect.entity.core-api/src/modules/integrations/entities/telegram-connect.entity.ts#L6-L6: Refactor this relative import to use@/modules/integrations/entities/integration.entity.🤖 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/modules/integrations/integrations.module.ts` around lines 4 - 10, Replace all relative imports with the `@/` alias in core-api/src/modules/integrations/integrations.module.ts lines 4-10, using module-root paths; update the Integration entity import in core-api/src/modules/integrations/entities/integration.entity.ts line 4 and the TelegramConnect entity import in core-api/src/modules/integrations/entities/telegram-connect.entity.ts line 6 similarly. Preserve the imported symbols and module behavior.Source: Coding guidelines
core-api/src/modules/integrations/dto/create-telegram-pairing.dto.ts (1)
3-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove
CreateTelegramPairingDto— it isn’t referenced anywhere, and the Telegram pairing endpoint already usesTelegramConnectDtofor its response shape.🤖 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/modules/integrations/dto/create-telegram-pairing.dto.ts` around lines 3 - 8, Remove the unused CreateTelegramPairingDto class and its file, ensuring the Telegram pairing endpoint continues using TelegramConnectDto for the response shape and no imports or exports reference the removed DTO.console/src/pages/integrations/components/telegram-connect.tsx (1)
36-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffHand-rolled DTO/status duplication instead of the generated API client.
This file re-declares
TelegramConnectDtoand comparesstatusagainst raw string literals ('CONNECTED','PENDING') rather than reusing the backend'sTelegramConnectStatusenum and the project's generated OpenAPI client (already used elsewhere, e.g.integration-detail-sheet.tsxvia@/services/apis/gen/queries). Any future field/enum rename on the backend DTO won't be caught by the type system here.Also applies to: 323-325
🤖 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 `@console/src/pages/integrations/components/telegram-connect.tsx` around lines 36 - 51, Replace the local TelegramConnectDto and raw status literals in the Telegram connection component with the generated OpenAPI client types and TelegramConnectStatus enum, reusing the existing generated API patterns from integration-detail-sheet.tsx. Update the affected status comparisons and data references to use those generated symbols so backend field or enum changes are type-checked.console/src/pages/integrations/components/integration-detail-sheet.tsx (1)
533-543: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDrop the unused
botUsernameprop at this call site
integration.configonly storesbotToken, so this expression is alwaysundefinedhere.TelegramConnectalready falls back to the username returned from the pairing API, so passing the config value is redundant.🤖 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 `@console/src/pages/integrations/components/integration-detail-sheet.tsx` around lines 533 - 543, Remove the unused botUsername prop and its integration.config expression from the TelegramConnect invocation in the integration detail component; leave the integrationId prop and existing rendering conditions unchanged.
🤖 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 `@console/src/pages/integrations/components/telegram-connect.tsx`:
- Around line 234-244: Update the expiry message in the Telegram connection
component to derive its text from the pairing DTO’s tokenExpiredAt value, or
reuse the established shared TTL constant instead of hardcoding “10 minutes.”
Keep the message localized to the existing !connected state and ensure it
remains accurate when the backend token lifetime changes.
In `@core-api/src/database/migrations/1785000000000-AddWorkspaceDEK.ts`:
- Around line 13-14: Replace the unconditional DELETE in the AddWorkspaceDEK
migration with a non-destructive backfill that preserves configurations
decryptable through the legacy decrypt path in AssetsService. Only remove
records when legacy decryption definitively fails; otherwise migrate or retain
them so the dek ? decryptWithDEK(...) : decrypt(...) fallback remains
functional.
In `@core-api/src/modules/agents/agents.service.ts`:
- Around line 246-251: Update the apiKey handling in getLLMConfigsWithProviders
to check config.apiKey before calling decryptWithDEK or decrypt, matching the
guard in toLLMConfigResponse. Preserve the '****' fallback for falsy keys and
only pass a decrypted key to maskApiKey when config.apiKey is present.
In `@core-api/src/modules/apikeys/apikeys.service.ts`:
- Line 84: Update the API-key rotation flow around updateEntity(false) to
execute revocation and creation/save within a single database transaction,
rolling back both writes on failure. Add a database constraint or partial unique
index enforcing at most one non-revoked key per (type, ref), while preserving
uniqueness of key values.
In `@core-api/src/modules/assets/assets.service.spec.ts`:
- Around line 81-82: Update the mockWorkspaceEncryptionService setup used by
AssetsService tests to include a getDEK method mock, returning the expected key
value or resolved result required by generateTagsWithAI and related tests. Keep
the existing mock initialization and test behavior unchanged apart from
providing this method.
In `@core-api/src/modules/integrations/integrations.controller.ts`:
- Around line 213-220: Update telegramWebhook and
TelegramWebhookService.processUpdate to accept and propagate integrationId as
the integration scope when processing the webhook update. Ensure the route
parameter influences the selected integration instead of being discarded, while
preserving the existing update handling behavior.
- Around line 255-263: The disconnectTelegramConnect handler should use the same
validated parameter DTO pattern as sibling routes. Replace the raw id and
connectId parameter bindings with validated route-parameter DTOs, preserving the
existing values passed to telegramConnectService.disconnect while ensuring
malformed identifiers produce a validation error.
- Around line 206-222: Add a shared-secret flow for Telegram webhooks: update
the setWebhook configuration to register a secret_token, then update
telegramWebhook to read X-Telegram-Bot-Api-Secret-Token and reject requests
whose value does not match the configured secret before calling
TelegramWebhookService.processUpdate. Reuse the existing configuration and
error-response conventions.
In `@core-api/src/modules/integrations/integrations.service.ts`:
- Around line 54-62: Update the Telegram webhook request in the surrounding try
block to enforce a finite timeout for the fetch call, using the project’s
established timeout or abort-signal pattern if available. Ensure the timeout
applies to both create and update flows without changing the existing request
method, headers, or body.
- Around line 52-60: Update the webhook registration flow around setWebhook to
generate and persist a per-integration secret_token, then include it in the
Telegram API request. In the public webhook handler, validate
X-Telegram-Bot-Api-Secret-Token against that integration’s stored secret and
reject mismatches or missing values; also use the integrationId path parameter
to load and validate the intended integration before processing the request.
In `@core-api/src/modules/integrations/telegram-connect.service.ts`:
- Around line 71-112: The createPairing flow’s findOne/delete/save sequence must
be serialized to prevent concurrent requests from creating multiple pending
tokens for the same user and integration. Update createPairing to execute the
lookup, deletion, token creation, and save within a transaction using a lock
scoped to userId and integrationId, preserving the existing reuse and
bot-validation behavior.
- Around line 280-295: Update the connectRepo.find query in getConnects to
include isActive: true alongside integrationId and userId, ensuring deactivated
Telegram connects are excluded while preserving the existing ordering and DTO
mapping.
In `@core-api/src/modules/integrations/telegram-polling.service.ts`:
- Around line 139-147: Update TelegramWebhookService.processUpdate to accept an
integration scope parameter, then pass integration.id from the polling service
call in the error-handling flow. Propagate the new parameter through the
method’s implementation and preserve existing update-processing behavior.
- Around line 99-103: Move the workspace DEK lookup and sensitive-config
decryption in pollBot inside the existing try/catch, or add equivalent error
handling around them, and log failures with the service’s established logger
before returning or propagating. Ensure DEK/decryption errors from pollBot are
no longer silently rejected while preserving the existing botToken validation
and polling behavior.
- Around line 19-31: Add a NestJS shutdown lifecycle hook to
TelegramPollingService that invokes its existing stop() method and aborts any
in-flight polling request through abortController. Ensure the setTimeout/sleep
polling loop observes the stopped state or abort signal so no further recursive
polling continues after application shutdown.
- Around line 113-121: Update pollOnce’s getUpdates fetch to use a client-side
timeout via the existing AbortSignal.timeout pattern, combining it with
this.abortController’s signal so either service shutdown or timeout aborts the
request. Preserve the current early-abort behavior and ensure the timeout
prevents pollLoop from being blocked by a hung request.
In `@core-api/src/modules/integrations/telegram-webhook.service.ts`:
- Around line 45-78: Update processUpdate to accept and propagate integrationId
from the controller and polling loop into
TelegramConnectService.confirmConnection. Before creating the connection,
validate that the token’s connect.integrationId matches the received
integrationId, and reject mismatches while preserving the existing valid-token
flow.
In `@core-api/src/modules/notifications/processors/notifications.processor.ts`:
- Around line 112-130: The notification processing flow should isolate failures
for each integration: move decryptSensitiveConfigFields into the per-integration
Promise.allSettled callback so a decryption error affects only that integration,
and handle runConnector results that return success: false by reporting them as
integration failures. Preserve processing for other integrations and log enough
integration context for both decryption and connector failures.
In `@core-api/src/services/workspace-encryption/workspace-encryption.service.ts`:
- Around line 69-84: Update the DEK backfill flow in onModuleInit to track
failed workspace IDs instead of only logging and continuing after ensureDEK
errors. Retry each failed workspace or, if failures remain, report their IDs and
propagate an error so module initialization does not succeed with an incomplete
backfill; preserve the completion log for successful workspaces.
- Around line 33-40: Update ensureDEK so DEK persistence is write-once:
condition the update on dek still being NULL (or re-check immediately before
writing) and preserve an existing DEK when another backfill has already assigned
one. Also bound or renew the lease used by backfillMissingDEKs so work cannot
continue unprotected after the 60-second lockTTL expires.
---
Nitpick comments:
In `@console/src/pages/integrations/components/integration-detail-sheet.tsx`:
- Around line 533-543: Remove the unused botUsername prop and its
integration.config expression from the TelegramConnect invocation in the
integration detail component; leave the integrationId prop and existing
rendering conditions unchanged.
In `@console/src/pages/integrations/components/telegram-connect.tsx`:
- Around line 36-51: Replace the local TelegramConnectDto and raw status
literals in the Telegram connection component with the generated OpenAPI client
types and TelegramConnectStatus enum, reusing the existing generated API
patterns from integration-detail-sheet.tsx. Update the affected status
comparisons and data references to use those generated symbols so backend field
or enum changes are type-checked.
In `@console/src/pages/settings/components/api-keys-settings.tsx`:
- Around line 76-82: Update CopyableValue to accept an optional actions or
children prop and render it beside the copy button within its existing centered
flex container. Move the Rotate button from the separate layout in
ApiKeysSettings into this prop, preserving its current handler, disabled state,
icon, and rotating label so Copy and Rotate remain side by side.
In `@core-api/src/common/utils/encryption.util.ts`:
- Around line 64-104: Update decrypt() in
core-api/src/common/utils/encryption.util.ts at lines 64-104 to use
allKeys[keyIndex] directly after validating the index, while preserving all-key
fallback behavior for the legacy unprefixed format. Apply the same indexed-key
selection in unwrapDEK() in
core-api/src/common/utils/workspace-encryption.util.ts at lines 46-82; both
indexed branches must avoid trial-decrypting every configured key.
- Around line 92-104: Update the encryption/decryption implementation around
decrypt() to use authenticated encryption for new ciphertext, preferably
AES-256-GCM with its authentication tag verified before returning plaintext. If
CBC compatibility must remain, add an encrypt-then-MAC covering the ciphertext
and verify it before decryption, while preserving an explicit legacy migration
path as needed; do not rely on decipher.final() or multi-key trial success as
authentication.
In `@core-api/src/common/utils/workspace-encryption.util.spec.ts`:
- Around line 35-59: Extend the “wrapDEK / unwrapDEK” tests to exercise
multi-key rotation: configure process.env.ENCRYPTION_KEYS with multiple
comma-separated keys, wrap a DEK using the initial key set, append a new key,
and verify the original wrapped value still unwraps to the same DEK through its
stored key index. Restore the environment variable after the test to avoid
affecting other cases.
In `@core-api/src/common/utils/workspace-encryption.util.ts`:
- Around line 32-39: Update wrapDEK to call parseEncryptionKeys only once,
derive the active encryption key and activeIndex from that shared result, and
use both when creating the cipher and formatting the return value. Avoid using
getActiveEncryptionKey if it would trigger a second parse.
In `@core-api/src/modules/integrations/dto/create-telegram-pairing.dto.ts`:
- Around line 3-8: Remove the unused CreateTelegramPairingDto class and its
file, ensuring the Telegram pairing endpoint continues using TelegramConnectDto
for the response shape and no imports or exports reference the removed DTO.
In `@core-api/src/modules/integrations/integrations.module.ts`:
- Around line 4-10: Replace all relative imports with the `@/` alias in
core-api/src/modules/integrations/integrations.module.ts lines 4-10, using
module-root paths; update the Integration entity import in
core-api/src/modules/integrations/entities/integration.entity.ts line 4 and the
TelegramConnect entity import in
core-api/src/modules/integrations/entities/telegram-connect.entity.ts line 6
similarly. Preserve the imported symbols and module behavior.
In `@core-api/src/modules/integrations/telegram-connect.service.ts`:
- Around line 40-46: Remove the static repository wiring from onModuleInit and
refactor TelegramConnector to receive the connect repository through Nest
dependency injection, storing it as instance state used by push(). Update its
provider/module registration and constructor dependencies so direct test
instantiation and resolution outside module bootstrap do not depend on global
mutable state.
- Around line 356-370: Remove the duplicated Telegram API logic by making
TelegramConnector.push use shared helpers from TelegramConnectService. In
core-api/src/modules/integrations/telegram-connect.service.ts lines 356-370,
retain or extract getConnectedChatIds for the connected-chat query; in lines
375-403, extract or reuse sendTelegramMessage for the Bot API request. In
core-api/src/modules/integrations/connectors/telegram.connector.ts lines 98-117
and 119-164, replace the inline query and send loop with those helpers, and
dispatch sends via Promise.allSettled while preserving existing timeout and
error-handling behavior.
In `@core-api/src/modules/integrations/telegram-webhook.service.ts`:
- Around line 45-78: Thread the integration scope through Telegram update
processing: update TelegramWebhookService.processUpdate to accept integrationId,
verify the resolved connection’s integrationId matches it before calling
confirmConnection, and reject mismatches. In
core-api/src/modules/integrations/integrations.controller.ts lines 213-220, pass
the extracted route integrationId; in
core-api/src/modules/integrations/telegram-polling.service.ts lines 139-147,
pass integration.id.
In `@core-api/src/modules/integrations/validators/integration.validator.ts`:
- Around line 139-154: Update the catch block in decryptSensitiveConfigFields to
log a warning when decryptWithDEK fails, including sufficient context to
diagnose the failed sensitive field. Preserve the existing fallback behavior by
leaving the original value unchanged after the warning.
In `@core-api/src/modules/workspaces/entities/workspace.entity.ts`:
- Around line 126-128: Update the dekAt property in the workspace entity to add
class-validator date validation with `@IsDate`(), while preserving its existing
optional and nullable behavior.
In `@core-api/src/modules/workspaces/workspaces.service.spec.ts`:
- Around line 165-166: The empty mockWorkspaceEncryptionService does not cover
the workspace DEK creation contract. Add a generateWrappedDEK mock to
mockWorkspaceEncryptionService, then update the createWorkspace tests to verify
the persisted workspace includes both dek and dekAt values returned by the mock;
keep all external dependencies mocked.
🪄 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: 2446d34f-08d9-4794-b97b-243a93e199a1
⛔ Files ignored due to path filters (4)
console/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlconsole/src/services/apis/gen/queries.tsis excluded by!**/gen/**package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
console/package.jsonconsole/src/components/common/copyable-value.tsxconsole/src/pages/integrations/components/integration-detail-sheet.tsxconsole/src/pages/integrations/components/telegram-connect.tsxconsole/src/pages/settings/components/api-keys-settings.tsxcore-api/example.envcore-api/src/common/enums/enum.tscore-api/src/common/utils/encryption.util.tscore-api/src/common/utils/workspace-encryption.util.spec.tscore-api/src/common/utils/workspace-encryption.util.tscore-api/src/database/1776960340581-migrations.tscore-api/src/database/migrations/1784101236391-AddTelegramConnects.tscore-api/src/database/migrations/1785000000000-AddWorkspaceDEK.tscore-api/src/modules/agents/agents.service.tscore-api/src/modules/apikeys/apikeys.service.tscore-api/src/modules/assets/assets.service.spec.tscore-api/src/modules/assets/assets.service.tscore-api/src/modules/auth/entities/user.entity.tscore-api/src/modules/integrations/connectors/telegram.connector.tscore-api/src/modules/integrations/dto/create-telegram-pairing.dto.tscore-api/src/modules/integrations/dto/telegram-connect.dto.tscore-api/src/modules/integrations/entities/integration.entity.tscore-api/src/modules/integrations/entities/telegram-connect.entity.tscore-api/src/modules/integrations/integrations.controller.tscore-api/src/modules/integrations/integrations.module.tscore-api/src/modules/integrations/integrations.service.tscore-api/src/modules/integrations/schemas/telegram.schema.tscore-api/src/modules/integrations/telegram-connect.service.tscore-api/src/modules/integrations/telegram-polling.service.tscore-api/src/modules/integrations/telegram-webhook.service.tscore-api/src/modules/integrations/validators/integration.validator.tscore-api/src/modules/notifications/processors/notifications.processor.tscore-api/src/modules/workspaces/entities/workspace.entity.tscore-api/src/modules/workspaces/workspaces.service.spec.tscore-api/src/modules/workspaces/workspaces.service.tscore-api/src/services/redis/distributed-lock.service.tscore-api/src/services/services.module.tscore-api/src/services/workspace-encryption/workspace-encryption.service.ts
💤 Files with no reviewable changes (2)
- core-api/src/services/redis/distributed-lock.service.ts
- core-api/src/database/1776960340581-migrations.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (20)
console/src/pages/integrations/components/telegram-connect.tsx (1)
234-244: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n "TOKEN_TTL_MS" core-api/src/modules/integrationsRepository: oasm-platform/open-asm
Length of output: 401
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- telegram-connect.tsx around target lines ---' sed -n '180,270p' console/src/pages/integrations/components/telegram-connect.tsx echo echo '--- search for tokenExpiredAt and Telegram connect DTOs ---' rg -n "tokenExpiredAt|TelegramConnect|telegram-connect" console/src core-api/src/modules/integrations -g '!**/dist/**' -g '!**/build/**'Repository: oasm-platform/open-asm
Length of output: 19870
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- file list ---' git ls-files console/src/pages/integrations/components/telegram-connect.tsx core-api/src/modules/integrations | sed -n '1,120p'Repository: oasm-platform/open-asm
Length of output: 2389
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,120p' console/src/pages/integrations/components/telegram-connect.tsxRepository: oasm-platform/open-asm
Length of output: 3438
Derive the expiry copy from the DTO.
Token expires in 10 minutes.duplicates the backend TTL; usetokenExpiredAtor a shared TTL constant so the UI stays aligned if the token lifetime changes.🤖 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 `@console/src/pages/integrations/components/telegram-connect.tsx` around lines 234 - 244, Update the expiry message in the Telegram connection component to derive its text from the pairing DTO’s tokenExpiredAt value, or reuse the established shared TTL constant instead of hardcoding “10 minutes.” Keep the message localized to the existing !connected state and ensure it remains accurate when the backend token lifetime changes.core-api/src/database/migrations/1785000000000-AddWorkspaceDEK.ts (1)
13-14: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Avoid destructive deletion if data is recoverable.
The migration unconditionally deletes all
agent_llm_configsciting "unrecoverable old keys". However, the updatedAssetsServiceexplicitly implements a legacy fallback (dek ? decryptWithDEK(...) : decrypt(...)) to decrypt old configurations.If the legacy
decryptmethod still works for these configs, this deletion causes unnecessary data loss for users—consider a data migration (backfill) instead. If the keys are truly unrecoverable and legacy decryption fails, thisDELETEoperation is justified, but the fallback inAssetsServicewill never successfully execute for old records and could be removed.🤖 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/database/migrations/1785000000000-AddWorkspaceDEK.ts` around lines 13 - 14, Replace the unconditional DELETE in the AddWorkspaceDEK migration with a non-destructive backfill that preserves configurations decryptable through the legacy decrypt path in AssetsService. Only remove records when legacy decryption definitively fails; otherwise migrate or retain them so the dek ? decryptWithDEK(...) : decrypt(...) fallback remains functional.core-api/src/modules/agents/agents.service.ts (1)
246-251: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Decrypt call runs before the
apiKeytruthiness guard, unliketoLLMConfigResponse.
decryptWithDEK(config.apiKey, dek)/decrypt(config.apiKey)executes unconditionally, thenconfig.apiKey ? ... : '****'is checked afterward only for masking. Ifconfig.apiKeywere ever falsy, the decrypt call throws before the guard can protect it, failing the entiregetLLMConfigsWithProvidersresponse for the whole workspace.toLLMConfigResponse(lines 136-139) already guards apiKey truthiness before decrypting — this method should follow the same pattern.🐛 Proposed fix
- const apiKey = dek - ? decryptWithDEK(config.apiKey, dek) - : decrypt(config.apiKey); - const apiKeyMasked = config.apiKey - ? this.maskApiKey(apiKey) - : '****'; + const apiKeyMasked = config.apiKey + ? this.maskApiKey( + dek ? decryptWithDEK(config.apiKey, dek) : decrypt(config.apiKey), + ) + : '****';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const apiKeyMasked = config.apiKey ? this.maskApiKey( dek ? decryptWithDEK(config.apiKey, dek) : decrypt(config.apiKey), ) : '****';🤖 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/modules/agents/agents.service.ts` around lines 246 - 251, Update the apiKey handling in getLLMConfigsWithProviders to check config.apiKey before calling decryptWithDEK or decrypt, matching the guard in toLLMConfigResponse. Preserve the '****' fallback for falsy keys and only pass a decrypted key to maskApiKey when config.apiKey is present.core-api/src/modules/apikeys/apikeys.service.ts (1)
84-84: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify whether an active-key uniqueness constraint already exists. rg -n -C3 'isRevoked|ApiKey|api_keys' \ core-api/src/database/migrations \ core-api/src/modules/apikeysRepository: oasm-platform/open-asm
Length of output: 22852
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== apikeys.service.ts (relevant method) ==\n' sed -n '60,90p' core-api/src/modules/apikeys/apikeys.service.ts printf '\n== apikey.entity.ts ==\n' sed -n '1,120p' core-api/src/modules/apikeys/entities/apikey.entity.ts printf '\n== migration search for api_keys constraints/indexes ==\n' rg -n -C2 'api_keys|isRevoked|revokedAt|UNIQUE|INDEX|constraint' core-api/src/database/migrationsRepository: oasm-platform/open-asm
Length of output: 50378
Make API-key rotation atomic.
updateEntity(false)still leaves the revoke/save pair non-atomic. A failed save can clear the current key, and concurrent rotations can create multiple active keys because the schema only enforces uniqueness onkey, not on the active(type, ref)pair. Wrap both writes in a transaction and add a DB constraint/index for one non-revoked key per(type, ref).🤖 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/modules/apikeys/apikeys.service.ts` at line 84, Update the API-key rotation flow around updateEntity(false) to execute revocation and creation/save within a single database transaction, rolling back both writes on failure. Add a database constraint or partial unique index enforcing at most one non-revoked key per (type, ref), while preserving uniqueness of key values.core-api/src/modules/assets/assets.service.spec.ts (1)
81-82: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Mock the
getDEKmethod to prevent test failures.
AssetsServicenow relies onthis.workspaceEncryption.getDEK(...). Initializing the mock as an empty object will cause runtime errors (TypeError: this.workspaceEncryption.getDEK is not a function) when testing methods likegenerateTagsWithAIthat invoke it.🔧 Proposed fix
- mockWorkspaceEncryptionService = {}; + mockWorkspaceEncryptionService = { + getDEK: jest.fn().mockResolvedValue('mock-dek'), + };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.mockWorkspaceEncryptionService = { getDEK: jest.fn().mockResolvedValue('mock-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/modules/assets/assets.service.spec.ts` around lines 81 - 82, Update the mockWorkspaceEncryptionService setup used by AssetsService tests to include a getDEK method mock, returning the expected key value or resolved result required by generateTagsWithAI and related tests. Keep the existing mock initialization and test behavior unchanged apart from providing this method.core-api/src/modules/integrations/integrations.controller.ts (3)
206-222: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== integrations.controller.ts ==\n' sed -n '1,280p' core-api/src/modules/integrations/integrations.controller.ts printf '\n== search telegram webhook / setWebhook / secret_token / processUpdate ==\n' rg -n "setWebhook|secret_token|telegramWebhook|processUpdate|X-Telegram-Bot-Api-Secret-Token|telegram bot webhook|telegram/webhook" core-api/src -S printf '\n== candidate services/controllers ==\n' fd -a "telegram" core-api/src | sed -n '1,200p'Repository: oasm-platform/open-asm
Length of output: 10476
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== integrations.controller.ts (focused slice) ==\n' sed -n '180,260p' core-api/src/modules/integrations/integrations.controller.ts printf '\n== telegram-related files ==\n' fd -a "telegram" core-api/src | sort printf '\n== processUpdate references ==\n' rg -n "processUpdate\\(" core-api/src -S printf '\n== setWebhook references ==\n' rg -n "setWebhook|secret_token|X-Telegram-Bot-Api-Secret-Token" core-api/src -SRepository: oasm-platform/open-asm
Length of output: 4217
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== integrations.service.ts (webhook registration slice) ==\n' sed -n '1,140p' core-api/src/modules/integrations/integrations.service.ts printf '\n== telegram-webhook.service.ts ==\n' sed -n '1,220p' core-api/src/modules/integrations/telegram-webhook.service.ts printf '\n== telegram-polling.service.ts slice ==\n' sed -n '1,220p' core-api/src/modules/integrations/telegram-polling.service.tsRepository: oasm-platform/open-asm
Length of output: 12899
Add a shared secret to the Telegram webhook.
setWebhookcurrently registers only the URL, so this public endpoint accepts arbitrary POSTs. Pass asecret_tokenwhen configuring the webhook and reject requests whoseX-Telegram-Bot-Api-Secret-Tokenheader doesn’t match.🤖 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/modules/integrations/integrations.controller.ts` around lines 206 - 222, Add a shared-secret flow for Telegram webhooks: update the setWebhook configuration to register a secret_token, then update telegramWebhook to read X-Telegram-Bot-Api-Secret-Token and reject requests whose value does not match the configured secret before calling TelegramWebhookService.processUpdate. Reuse the existing configuration and error-response conventions.
213-220: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
integrationIdroute param is captured but discarded.Root cause is in
TelegramWebhookService.processUpdate, which doesn't accept an integration scope (see comment there); here it means the per-integration webhook URL segment has no effect on what gets processed.🤖 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/modules/integrations/integrations.controller.ts` around lines 213 - 220, Update telegramWebhook and TelegramWebhookService.processUpdate to accept and propagate integrationId as the integration scope when processing the webhook update. Ensure the route parameter influences the selected integration instead of being discarded, while preserving the existing update handling behavior.
255-263: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Inconsistent param validation vs. sibling endpoints.
Every other
:idroute in this file binds via@Param() { id }: IdQueryParamDto(presumably with format validation). This handler uses raw@Param('id') id: string/@Param('connectId') connectId: string, skipping that validation — a malformed id would likely surface as an unhandled DB error (500) instead of a clean 400.🤖 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/modules/integrations/integrations.controller.ts` around lines 255 - 263, The disconnectTelegramConnect handler should use the same validated parameter DTO pattern as sibling routes. Replace the raw id and connectId parameter bindings with validated route-parameter DTOs, preserving the existing values passed to telegramConnectService.disconnect while ensuring malformed identifiers produce a validation error.core-api/src/modules/integrations/integrations.service.ts (2)
52-60: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== integrations service outline ==\n' ast-grep outline core-api/src/modules/integrations/integrations.service.ts --view expanded || true printf '\n== search for telegram webhook references ==\n' rg -n "telegram|setWebhook|secret_token|X-Telegram-Bot-Api-Secret-Token|integrationId" core-api/src/modules -S || true printf '\n== candidate files ==\n' fd -a ".*telegram.*|.*integrat.*controller.*|.*webhook.*" core-api/src/modules -E node_modules || trueRepository: oasm-platform/open-asm
Length of output: 16930
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' core-api/src/modules/integrations/integrations.service.tsRepository: oasm-platform/open-asm
Length of output: 7607
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== integrations controller outline ==\n' ast-grep outline core-api/src/modules/integrations/integrations.controller.ts --view expanded || true printf '\n== telegram webhook service outline ==\n' ast-grep outline core-api/src/modules/integrations/telegram-webhook.service.ts --view expanded || true printf '\n== controller webhook handler ==\n' sed -n '200,235p' core-api/src/modules/integrations/integrations.controller.ts printf '\n== telegram webhook service ==\n' sed -n '1,220p' core-api/src/modules/integrations/telegram-webhook.service.tsRepository: oasm-platform/open-asm
Length of output: 4020
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import urllib.request, urllib.parse, json, re # We'll check Telegram docs page title/snippet via web is not available here. # Placeholder to keep shell-only evidence focused on code paths. print("shell probe only") PYRepository: oasm-platform/open-asm
Length of output: 177
🌐 Web query:
Telegram Bot API setWebhook secret_token X-Telegram-Bot-Api-Secret-Token documentation💡 Result:
The secret_token parameter in the Telegram Bot API setWebhook method is an optional security feature used to authenticate incoming webhook requests [1]. By providing a secret_token during the setWebhook call, you instruct Telegram to include that specific string in the X-Telegram-Bot-Api-Secret-Token HTTP header of every subsequent update sent to your webhook URL [1]. Key documentation details: - Purpose: It allows you to verify that the request truly originates from a webhook set by you, preventing unauthorized parties from sending spoofed updates to your endpoint [1][2]. - Format requirements: The token must be a string of 1 to 256 characters [1]. - Allowed characters: Only characters A-Z, a-z, 0-9, _ (underscore), and - (hyphen) are permitted [1]. - Implementation: When your server receives a POST request at your webhook URL, you should extract the value of the X-Telegram-Bot-Api-Secret-Token header and compare it against the secret token you originally provided [2]. If the values do not match, you should reject the request, typically by returning an HTTP 403 Forbidden status code [2]. This feature was introduced to provide a standard, secure way to validate webhook authenticity [3].
Citations:
- 1: https://core.telegram.org/bots/API
- 2: https://nguyenthanhluan.com/en/glossary/secret_token-for-setwebhook-en/
- 3: https://stackoverflow.com/questions/69882004/how-verify-request-of-webhook-are-from-telegram
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' core-api/src/modules/integrations/telegram-webhook.service.tsRepository: oasm-platform/open-asm
Length of output: 2188
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' core-api/src/modules/integrations/schemas/telegram.schema.tsRepository: oasm-platform/open-asm
Length of output: 1541
🌐 Web query:
site:core.telegram.org bots api setWebhook secret_token X-Telegram-Bot-Api-Secret-Token💡 Result:
In the Telegram Bot API, the secret_token parameter for the setWebhook method is an optional string used to enhance the security of your webhook integration [1][2]. By providing this token when you set your webhook, Telegram will include it as the value of the X-Telegram-Bot-Api-Secret-Token header in every HTTPS POST request sent to your webhook URL [1][3]. This feature allows your server to verify that the incoming requests are indeed originating from the specific webhook you configured, protecting your endpoint from unauthorized or spoofed traffic [1][2]. Key details regarding the secret_token: - Length: 1-256 characters [1][4]. - Allowed Characters: A-Z, a-z, 0-9, underscores (_), and hyphens (-) [1][2]. - Usage: It is specified during the setWebhook API call [1][3]. When your server receives a request from Telegram, you should check that the X-Telegram-Bot-Api-Secret-Token header matches the token you provided during setup [1][2].
Citations:
- 1: https://core.telegram.org/bots/API
- 2: https://core.telegram.org/bots/api?referrer=wordpress.com
- 3: https://core.telegram.org/bots/api%20
- 4: https://core.telegram.org/bots/api%20%20
🏁 Script executed:
#!/bin/bash set -euo pipefail ast-grep outline core-api/src/modules/integrations/telegram-connect.service.ts --view expanded || true sed -n '1,260p' core-api/src/modules/integrations/telegram-connect.service.tsRepository: oasm-platform/open-asm
Length of output: 9782
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '260,460p' core-api/src/modules/integrations/telegram-connect.service.tsRepository: oasm-platform/open-asm
Length of output: 5449
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,260p' core-api/src/modules/integrations/telegram-connect.service.tsRepository: oasm-platform/open-asm
Length of output: 9080
🏁 Script executed:
#!/bin/bash set -euo pipefail ast-grep outline core-api/src/modules/integrations/telegram-connect.service.ts --view expanded || true rg -n "confirmConnection|createPairing|token" core-api/src/modules/integrations/telegram-connect.service.ts -n -A40 -B20Repository: oasm-platform/open-asm
Length of output: 10396
Authenticate Telegram webhook deliveries. Register a per-integration
secret_tokeninsetWebhookand reject requests unlessX-Telegram-Bot-Api-Secret-Tokenmatches; the public webhook route currently accepts any POST and ignores theintegrationIdpath param.🤖 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/modules/integrations/integrations.service.ts` around lines 52 - 60, Update the webhook registration flow around setWebhook to generate and persist a per-integration secret_token, then include it in the Telegram API request. In the public webhook handler, validate X-Telegram-Bot-Api-Secret-Token against that integration’s stored secret and reject mismatches or missing values; also use the integrationId path parameter to load and validate the intended integration before processing the request.
54-62: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the target file around the relevant lines. sed -n '1,180p' core-api/src/modules/integrations/integrations.service.ts printf '\n--- SEARCH timeout patterns ---\n' rg -n "AbortSignal\.timeout|AbortController|timeout\\(" core-api/src -g '!**/dist/**' -g '!**/build/**' printf '\n--- Search fetch usage in core-api ---\n' rg -n "\\bfetch\\(" core-api/src -g '!**/dist/**' -g '!**/build/**'Repository: oasm-platform/open-asm
Length of output: 9166
Add a timeout to the Telegram webhook request.
fetchcan hang indefinitely here, so repeated create/update calls may leave requests pending.Proposed timeout
{ method: 'POST', + signal: AbortSignal.timeout(10_000), headers: { 'Content-Type': 'application/json' },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.try { const res = await fetch( `https://api.telegram.org/bot${botToken}/setWebhook`, { method: 'POST', signal: AbortSignal.timeout(10_000), headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ url: webhookUrl }), }, );🤖 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/modules/integrations/integrations.service.ts` around lines 54 - 62, Update the Telegram webhook request in the surrounding try block to enforce a finite timeout for the fetch call, using the project’s established timeout or abort-signal pattern if available. Ensure the timeout applies to both create and update flows without changing the existing request method, headers, or body.core-api/src/modules/integrations/telegram-connect.service.ts (2)
71-112: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Serialize pairing token creation. The
findOne→delete→insertflow can race under concurrentcreatePairingcalls, so two requests can mint different pending tokens for the same user/integration. Wrap the sequence in a transaction with a per-user+integration lock, or add a unique constraint and handle the conflict.🤖 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/modules/integrations/telegram-connect.service.ts` around lines 71 - 112, The createPairing flow’s findOne/delete/save sequence must be serialized to prevent concurrent requests from creating multiple pending tokens for the same user and integration. Update createPairing to execute the lookup, deletion, token creation, and save within a transaction using a lock scoped to userId and integrationId, preserving the existing reuse and bot-validation behavior.
280-295: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,380p' core-api/src/modules/integrations/telegram-connect.service.tsRepository: oasm-platform/open-asm
Length of output: 12799
🏁 Script executed:
ast-grep outline core-api/src/modules/integrations/entities/telegram-connect.entity.ts --view expandedRepository: oasm-platform/open-asm
Length of output: 824
🏁 Script executed:
rg -n "isActive|telegram-connect" core-api/src/modules/integrations -g '*.ts'Repository: oasm-platform/open-asm
Length of output: 2455
🏁 Script executed:
sed -n '390,470p' core-api/src/modules/integrations/telegram-connect.service.ts && printf '\n---\n' && sed -n '1,120p' core-api/src/modules/integrations/entities/telegram-connect.entity.ts && printf '\n---\n' && sed -n '1,120p' core-api/src/modules/integrations/dto/telegram-connect.dto.ts && printf '\n---\n' && sed -n '1,220p' core-api/src/modules/integrations/connectors/telegram.connector.tsRepository: oasm-platform/open-asm
Length of output: 9544
Filter inactive Telegram connects here too
getConnectsshould addisActive: trueto match the other Telegram queries; otherwise deactivated rows can still be returned here.🤖 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/modules/integrations/telegram-connect.service.ts` around lines 280 - 295, Update the connectRepo.find query in getConnects to include isActive: true alongside integrationId and userId, ensuring deactivated Telegram connects are excluded while preserving the existing ordering and DTO mapping.core-api/src/modules/integrations/telegram-polling.service.ts (4)
19-31: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
No shutdown hook to stop the polling loop.
The class implements
OnApplicationBootstrapto start polling but nothing callsstop()on shutdown. The recursivesetTimeout-based loop (viasleep) and any in-flight requests keep running past a graceful shutdown signal.🛑 Suggested fix
-export class TelegramPollingService implements OnApplicationBootstrap { +export class TelegramPollingService implements OnApplicationBootstrap, OnModuleDestroy { ... + onModuleDestroy(): void { + this.stop(); + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.`@Injectable`() export class TelegramPollingService implements OnApplicationBootstrap, OnModuleDestroy { private readonly logger = new Logger(TelegramPollingService.name); private active = false; private abortController?: AbortController; constructor( `@InjectRepository`(Integration) private readonly integrationRepo: Repository<Integration>, private readonly redisLockService: RedisLockService, private readonly telegramWebhookService: TelegramWebhookService, private readonly workspaceEncryption: WorkspaceEncryptionService, ) {} onModuleDestroy(): void { this.stop(); }🤖 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/modules/integrations/telegram-polling.service.ts` around lines 19 - 31, Add a NestJS shutdown lifecycle hook to TelegramPollingService that invokes its existing stop() method and aborts any in-flight polling request through abortController. Ensure the setTimeout/sleep polling loop observes the stopped state or abort signal so no further recursive polling continues after application shutdown.
99-103: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
DEK/decrypt failures are silently swallowed with no logging.
These two calls run outside the
try/catchbelow, andpollBot's only caller wraps it inPromise.allSettledwithout inspecting rejected results — so a DEK lookup or decryption failure for a bot produces zero log output, every poll cycle, making it effectively invisible.🔍 Suggested fix
private async pollBot(integration: Integration): Promise<void> { - const dek = await this.workspaceEncryption.getDEK(integration.workspaceId); - const config = decryptSensitiveConfigFields(integration.config, dek); + let config: Record<string, unknown>; + try { + const dek = await this.workspaceEncryption.getDEK(integration.workspaceId); + config = decryptSensitiveConfigFields(integration.config, dek); + } catch (err) { + this.logger.error(`DEK/decrypt failure for integration ${integration.id}`, err); + return; + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.private async pollBot(integration: Integration): Promise<void> { let config: Record<string, unknown>; try { const dek = await this.workspaceEncryption.getDEK(integration.workspaceId); config = decryptSensitiveConfigFields(integration.config, dek); } catch (err) { this.logger.error(`DEK/decrypt failure for integration ${integration.id}`, err); return; } const botToken = config.botToken as string | undefined; if (!botToken) return;🤖 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/modules/integrations/telegram-polling.service.ts` around lines 99 - 103, Move the workspace DEK lookup and sensitive-config decryption in pollBot inside the existing try/catch, or add equivalent error handling around them, and log failures with the service’s established logger before returning or propagating. Ensure DEK/decryption errors from pollBot are no longer silently rejected while preserving the existing botToken validation and polling behavior.
113-121: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the target service around the cited lines sed -n '1,220p' core-api/src/modules/integrations/telegram-polling.service.ts printf '\n--- SEARCH AbortSignal.timeout USAGE ---\n' rg -n "AbortSignal\.timeout|AbortSignal\.any|fetch\(" core-api/src/modules/integrations/telegram-connect.service.ts core-api/src/modules/integrations/telegram.connector.ts core-api/src/modules/integrations/telegram-polling.service.ts printf '\n--- POLLING ORCHESTRATION SEARCH ---\n' rg -n "Promise\.allSettled|pollOnce|pollLoop|setInterval|while \(true\)|for await|await .*pollOnce" core-api/src/modules/integrations/telegram-polling.service.tsRepository: oasm-platform/open-asm
Length of output: 6377
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- telegram-connect.service.ts (relevant slices) ---' sed -n '100,150p' core-api/src/modules/integrations/telegram-connect.service.ts printf '%s\n' '---' sed -n '360,410p' core-api/src/modules/integrations/telegram-connect.service.ts printf '%s\n' '--- polling flow with line numbers ---' nl -ba core-api/src/modules/integrations/telegram-polling.service.ts | sed -n '80,140p'Repository: oasm-platform/open-asm
Length of output: 3309
Add a client-side timeout to
getUpdates.
timeout=only limits Telegram’s server-side long poll; if the request hangs,pollOnce()waits onPromise.allSettled()andpollLoop()can’t start the next cycle, so one stuck bot blocks polling for all integrations. Use the sameAbortSignal.timeout(...)pattern as the other Telegram fetches, combined with the service abort signal.🤖 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/modules/integrations/telegram-polling.service.ts` around lines 113 - 121, Update pollOnce’s getUpdates fetch to use a client-side timeout via the existing AbortSignal.timeout pattern, combining it with this.abortController’s signal so either service shutdown or timeout aborts the request. Preserve the current early-abort behavior and ensure the timeout prevents pollLoop from being blocked by a hung request.
139-147: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
integration.idis available here but not passed toprocessUpdate.Root cause is
TelegramWebhookService.processUpdate's signature (doesn't accept an integration scope); this call site hasintegration.idon hand and could pass it once that's added.🤖 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/modules/integrations/telegram-polling.service.ts` around lines 139 - 147, Update TelegramWebhookService.processUpdate to accept an integration scope parameter, then pass integration.id from the polling service call in the error-handling flow. Propagate the new parameter through the method’s implementation and preserve existing update-processing behavior.core-api/src/modules/integrations/telegram-webhook.service.ts (1)
45-78: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the relevant files and inspect the surrounding code. git ls-files 'core-api/src/modules/integrations/*' 'core-api/src/modules/**/telegram*' | sed -n '1,200p' printf '\n--- telegram-webhook.service.ts ---\n' sed -n '1,220p' core-api/src/modules/integrations/telegram-webhook.service.ts printf '\n--- telegram-connect.service.ts ---\n' sed -n '1,260p' core-api/src/modules/integrations/telegram-connect.service.ts printf '\n--- controller/polling references ---\n' rg -n "integrationId|confirmConnection\\(|telegramConnectService|telegram-webhook" core-api/src/modules -g '!**/*.map'Repository: oasm-platform/open-asm
Length of output: 18494
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n--- integrations.controller.ts (webhook + pairing routes) ---\n' sed -n '180,235p' core-api/src/modules/integrations/integrations.controller.ts printf '\n--- telegram-polling.service.ts ---\n' sed -n '1,220p' core-api/src/modules/integrations/telegram-polling.service.ts printf '\n--- telegram-connect.entity.ts ---\n' sed -n '1,220p' core-api/src/modules/integrations/entities/telegram-connect.entity.ts printf '\n--- telegram.connector.ts (integrationId expectations) ---\n' sed -n '1,160p' core-api/src/modules/integrations/connectors/telegram.connector.tsRepository: oasm-platform/open-asm
Length of output: 14324
processUpdateshould carry the integration context through The controller and polling loop already knowintegrationId, but it gets dropped beforeconfirmConnection, so any valid/start <token>is accepted without checking which bot received it. ThreadintegrationIdthrough and compare it withconnect.integrationIdbefore creating the connection.🤖 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/modules/integrations/telegram-webhook.service.ts` around lines 45 - 78, Update processUpdate to accept and propagate integrationId from the controller and polling loop into TelegramConnectService.confirmConnection. Before creating the connection, validate that the token’s connect.integrationId matches the received integrationId, and reject mismatches while preserving the existing valid-token flow.core-api/src/modules/notifications/processors/notifications.processor.ts (1)
112-130: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Isolate and report per-integration failures.
Decryption currently occurs before
Promise.allSettled, so one corrupt config aborts the whole batch. Also,runConnectorcan returnsuccess: false, which is currently treated as fulfilled and never logged.Proposed failure isolation
- const enabledIntegrations = integrations - .map((integration) => ({ - integration, - config: decryptSensitiveConfigFields(integration.config, dek), - })) - .filter(({ config }) => config[type] !== false); - const results = await Promise.allSettled( - enabledIntegrations.map(async ({ integration, config }) => { + integrations.map(async (integration) => { + const config = decryptSensitiveConfigFields(integration.config, dek); + if (config[type] === false) return; + const pushConfig: Record<string, unknown> = { ...config, text: message, metadata, workspaceId, type, integrationId: integration.id, }; - await runConnector(integration.appType, integration.category, pushConfig); + const result = await runConnector( + integration.appType, + integration.category, + pushConfig, + ); + if (!result.success) { + throw new Error(result.error ?? result.message); + } }), );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const results = await Promise.allSettled( integrations.map(async (integration) => { const config = decryptSensitiveConfigFields(integration.config, dek); if (config[type] === false) return; const pushConfig: Record<string, unknown> = { ...config, text: message, metadata, workspaceId, type, integrationId: integration.id, }; const result = await runConnector( integration.appType, integration.category, pushConfig, ); if (!result.success) { throw new Error(result.error ?? result.message); } }), );🤖 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/modules/notifications/processors/notifications.processor.ts` around lines 112 - 130, The notification processing flow should isolate failures for each integration: move decryptSensitiveConfigFields into the per-integration Promise.allSettled callback so a decryption error affects only that integration, and handle runConnector results that return success: false by reporting them as integration failures. Preserve processing for other integrations and log enough integration context for both decryption and connector failures.core-api/src/services/workspace-encryption/workspace-encryption.service.ts (2)
33-40: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the relevant files first git ls-files 'core-api/src/services/workspace-encryption/workspace-encryption.service.ts' \ 'core-api/src/services/redis/distributed-lock.service.ts' \ 'core-api/src/**/workspace*.ts' \ 'core-api/src/**/redis*.ts' printf '\n--- outline: workspace-encryption.service.ts ---\n' ast-grep outline core-api/src/services/workspace-encryption/workspace-encryption.service.ts --view expanded || true printf '\n--- outline: distributed-lock.service.ts ---\n' ast-grep outline core-api/src/services/redis/distributed-lock.service.ts --view expanded || true printf '\n--- relevant search: ensureDEK / backfill / lock TTL ---\n' rg -n "ensureDEK|backfillMissingDEKs|withLock|lockTTL|workspace-dek-backfill|dekAt|IsNull\\(" core-api/src/services -SRepository: oasm-platform/open-asm
Length of output: 4636
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' core-api/src/services/workspace-encryption/workspace-encryption.service.ts printf '\n--- distributed-lock.service.ts ---\n' sed -n '1,220p' core-api/src/services/redis/distributed-lock.service.tsRepository: oasm-platform/open-asm
Length of output: 8107
🏁 Script executed:
#!/bin/bash set -euo pipefail # Print the relevant sections with line numbers cat -n core-api/src/services/workspace-encryption/workspace-encryption.service.ts | sed -n '1,260p' printf '\n--- distributed-lock.service.ts ---\n' cat -n core-api/src/services/redis/distributed-lock.service.ts | sed -n '1,260p'Repository: oasm-platform/open-asm
Length of output: 9871
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path paths = [ Path('core-api/src/services/workspace-encryption/workspace-encryption.service.ts'), Path('core-api/src/services/redis/distributed-lock.service.ts'), ] for p in paths: print(f"\n===== {p} =====") text = p.read_text() lines = text.splitlines() for i, line in enumerate(lines, 1): if any(k in line for k in ['ensureDEK', 'backfillMissingDEKs', 'withLock', 'workspace-dek-backfill', 'dekAt', 'IsNull', 'update(']): start = max(1, i-12) end = min(len(lines), i+20) for j in range(start, end+1): print(f"{j:4d}: {lines[j-1]}") print() PYRepository: oasm-platform/open-asm
Length of output: 13799
🌐 Web query:
site:github.com/oasm-platform/open-asm "workspace-encryption.service.ts" "ensureDEK" "backfillMissingDEKs"💡 Result:
The functions ensureDEK and backfillMissingDEKs are associated with workspace encryption management in the Open Attack Surface Management (OASM) platform [1][2]. Specifically, within the codebase (typically located in services such as workspace-encryption.service.ts), these methods are designed to maintain the integrity of Data Encryption Keys (DEKs) [1]. - ensureDEK: This function is generally responsible for verifying that a workspace has a valid, operational DEK assigned to it. If the key is missing or invalid, it ensures one is generated or retrieved to maintain secure operations [1]. - backfillMissingDEKs: This utility function is designed for maintenance and data migration tasks. It iterates through existing workspaces or encrypted assets that may lack a properly associated DEK—often due to system upgrades, refactors, or new security requirements—and applies the necessary keys to ensure data remains decryptable and compliant with the platform's security policies [1]. These mechanisms are part of the platform's broader approach to securing sensitive data, such as API keys and other PII, often utilizing AES-256-CBC or similar encryption standards [1]. If you are encountering issues related to these functions, it is likely due to a migration gap where older data was not correctly linked to the updated encryption service logic [1].
Citations:
🌐 Web query:
"oasm-platform/open-asm" "workspace-encryption.service.ts" "workspace-dek-backfill"💡 Result:
The requested file
workspace-encryption.service.tsand the termworkspace-dek-backfilldo not exist within theoasm-platform/open-asmrepository [1][2]. The OASM (Open Attack Surface Management) platform is an open-source cybersecurity project built with a NestJS backend (core-api), React frontend (console), and Go-based workers [1][3][2]. While it manages multi-workspace security assets and uses various services for database operations, job management, and agent memory [1][2][4], there is no evidence of an encryption-specific service with that name or a corresponding data encryption key (DEK) backfill utility in its codebase [1][2]. It is possible the query refers to code from a different project, such astwentyhq/twenty, which has recently implemented aSecretEncryptionService.tsand related versioned encryption features involvingworkspaceIdcontext and environment-based encryption keys [5]. Users searching for encryption patterns in a TypeScript-based NestJS architecture may be conflating these projects [5][2].Citations:
- 1: https://github.com/oasm-platform/open-asm
- 2: https://github.com/oasm-platform/open-asm/blob/main/AGENTS.md
- 3: https://github.com/oasm-platform/open-asm/blob/main/README.md
- 4: #428
- 5: twentyhq/twenty#20528
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import urllib.request, sys urls = [ 'https://raw.githubusercontent.com/oasm-platform/open-asm/feat/workspace-level-envelope-encryption-with-key-rotation/core-api/src/services/workspace-encryption/workspace-encryption.service.ts', 'https://raw.githubusercontent.com/oasm-platform/open-asm/feat/workspace-level-envelope-encryption-with-key-rotation/core-api/src/services/redis/distributed-lock.service.ts', ] for url in urls: print(f"\n===== {url} =====") try: with urllib.request.urlopen(url, timeout=20) as r: data = r.read().decode('utf-8') for needle in ['ensureDEK', 'backfillMissingDEKs', 'withLock', 'workspace-dek-backfill', 'dekAt', 'IsNull', 'update(']: if needle in data: print(f"found {needle}") # print concise surrounding context if available lines = data.splitlines() for idx, line in enumerate(lines, 1): if any(k in line for k in ['ensureDEK', 'backfillMissingDEKs', 'withLock', 'workspace-dek-backfill', 'dekAt', 'IsNull', 'update(']): start=max(1, idx-8); end=min(len(lines), idx+12) for j in range(start, end+1): print(f"{j:4d}: {lines[j-1]}") print() break except Exception as e: print(f"ERROR: {e}") PYRepository: oasm-platform/open-asm
Length of output: 831
Make DEK assignment write-once
ensureDEKstill overwritesdekunconditionally, so if the 60s backfill lease expires and another instance starts, it can replace a DEK already written by the first pass. Use a conditional update (dek IS NULL) or otherwise re-check before writing, and renew/bound the backfill work.🤖 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 33 - 40, Update ensureDEK so DEK persistence is write-once: condition the update on dek still being NULL (or re-check immediately before writing) and preserve an existing DEK when another backfill has already assigned one. Also bound or renew the lease used by backfillMissingDEKs so work cannot continue unprotected after the 60-second lockTTL expires.
69-84: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not silently complete an incomplete DEK backfill.
Failures are swallowed, so
onModuleInitsucceeds while affected workspaces remain without DEKs indefinitely. Retry failures or fail startup after reporting the failed workspace IDs.🤖 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 69 - 84, Update the DEK backfill flow in onModuleInit to track failed workspace IDs instead of only logging and continuing after ensureDEK errors. Retry each failed workspace or, if failures remain, report their IDs and propagate an error so module initialization does not succeed with an incomplete backfill; preserve the completion log for successful workspaces.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core-api/src/modules/agents/agents.completions.ts (1)
238-241: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify decryption fallback logic.
decryptWithDEKnatively handles the fallback to legacy decryption whendekisnull(or when the payload uses the legacy format). You can safely remove the ternary condition and calldecryptWithDEKdirectly.(If the
decryptutility is unused elsewhere in this file after this change, its import can also be safely removed).♻️ Proposed refactor
- if (config.apiKey) { - const dek = await this.workspaceEncryption.getDEK(workspaceId); - apiKey = dek ? decryptWithDEK(config.apiKey, dek) : decrypt(config.apiKey); - } + if (config.apiKey) { + const dek = await this.workspaceEncryption.getDEK(workspaceId); + apiKey = decryptWithDEK(config.apiKey, 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/modules/agents/agents.completions.ts` around lines 238 - 241, In the config.apiKey handling block, update the decryption call to use decryptWithDEK directly and let it perform its built-in legacy fallback when dek is unavailable or the payload is legacy-formatted. Remove the decrypt import if it is no longer referenced elsewhere in the file.
🤖 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.
Nitpick comments:
In `@core-api/src/modules/agents/agents.completions.ts`:
- Around line 238-241: In the config.apiKey handling block, update the
decryption call to use decryptWithDEK directly and let it perform its built-in
legacy fallback when dek is unavailable or the payload is legacy-formatted.
Remove the decrypt import if it is no longer referenced elsewhere in the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1eaab9e3-9c48-47b0-bbb9-9938cd31fb62
📒 Files selected for processing (1)
core-api/src/modules/agents/agents.completions.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core-api/src/services/workspace-encryption/workspace-encryption.service.ts (1)
97-103: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winOnly assign a DEK while the workspace still has none.
The backfill reads null rows and later performs an unconditional update. A concurrent DEK assignment between those operations is overwritten, making ciphertext encrypted with that DEK permanently unreadable. Update with
{ id: workspaceId, dek: IsNull() }, inspectaffected, and cache the generated key only when this update wins.🤖 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 97 - 103, Update ensureDEK to perform a conditional workspaceRepo.update using workspaceId and dek: IsNull(), then inspect the update result’s affected count. Cache the generated DEK only when the conditional update affects a row; otherwise preserve the existing DEK and do not cache the newly generated key.
🤖 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 28-30: Remove the encryptCache and decryptCache fields and all
related memoization in the workspace encryption service. Update the encryption
and decryption methods to always perform randomized encryption and
workspace-specific DEK validation, while retaining only the dekCache for
unwrapped DEKs.
- Line 104: Update the DEK caching flow around ensureDEK so generated keys are
inserted through the same bounded-cache path as other entries, invoking
evictIfNeeded after the cache update. Preserve existing DEK generation and
retrieval behavior while ensuring backfill cannot retain unbounded raw DEKs.
- Around line 33-37: Update WorkspaceEncryptionService.evictIfNeeded to obtain
the oldest map key through a single iterator iteration rather than assigning
map.keys().next().value directly, avoiding the inferred any type while
preserving the existing conditional deletion behavior.
---
Outside diff comments:
In `@core-api/src/services/workspace-encryption/workspace-encryption.service.ts`:
- Around line 97-103: Update ensureDEK to perform a conditional
workspaceRepo.update using workspaceId and dek: IsNull(), then inspect the
update result’s affected count. Cache the generated DEK only when the
conditional update affects a row; otherwise preserve the existing DEK and do not
cache the newly generated key.
🪄 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: d7e1daa2-4c7e-4cb7-9c56-ad74f9372df0
📒 Files selected for processing (4)
core-api/src/modules/agents/agents.completions.tscore-api/src/modules/agents/agents.service.tscore-api/src/modules/assets/assets.service.tscore-api/src/services/workspace-encryption/workspace-encryption.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- core-api/src/modules/agents/agents.service.ts
| private readonly dekCache = new Map<string, Buffer>(); | ||
| private readonly encryptCache = new Map<string, string>(); | ||
| private readonly decryptCache = new Map<string, string>(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not memoize encryption or decrypted secret values.
encryptCache retains plaintext as keys and reuses ciphertext instead of preserving randomized encryption. decryptCache retains plaintext values and is keyed only by ciphertext, so a request from another workspace can bypass DEK validation after a cache hit. Remove both result caches; caching only unwrapped DEKs is sufficient.
Also applies to: 144-168
🤖 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 28 - 30, Remove the encryptCache and decryptCache fields and all
related memoization in the workspace encryption service. Update the encryption
and decryption methods to always perform randomized encryption and
workspace-specific DEK validation, while retaining only the dekCache for
unwrapped DEKs.
| private evictIfNeeded<K, V>(map: Map<K, V>): void { | ||
| if (map.size >= WorkspaceEncryptionService.MAX_CACHE_SIZE) { | ||
| const oldest = map.keys().next().value; | ||
| if (oldest !== undefined) map.delete(oldest); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Avoid the unsafe iterator-value assignment.
Line 35 fails the current lint check because .value is inferred as any. Iterate once instead.
Proposed fix
private evictIfNeeded<K, V>(map: Map<K, V>): void {
if (map.size >= WorkspaceEncryptionService.MAX_CACHE_SIZE) {
- const oldest = map.keys().next().value;
- if (oldest !== undefined) map.delete(oldest);
+ for (const oldest of map.keys()) {
+ map.delete(oldest);
+ break;
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private evictIfNeeded<K, V>(map: Map<K, V>): void { | |
| if (map.size >= WorkspaceEncryptionService.MAX_CACHE_SIZE) { | |
| const oldest = map.keys().next().value; | |
| if (oldest !== undefined) map.delete(oldest); | |
| } | |
| private evictIfNeeded<K, V>(map: Map<K, V>): void { | |
| if (map.size >= WorkspaceEncryptionService.MAX_CACHE_SIZE) { | |
| for (const oldest of map.keys()) { | |
| map.delete(oldest); | |
| break; | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Check Lint / 1_Check Lint (core-api).txt
[error] 35-35: ESLint (@typescript-eslint/no-unsafe-assignment): Unsafe assignment of an any value.
🪛 GitHub Actions: Check Lint / Check Lint (core-api)
[error] 35-35: ESLint (@typescript-eslint/no-unsafe-assignment): Unsafe assignment of an any value.
🪛 GitHub Check: Check Lint (core-api)
[failure] 35-35:
Unsafe assignment of an any value
🤖 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 33 - 37, Update WorkspaceEncryptionService.evictIfNeeded to obtain
the oldest map key through a single iterator iteration rather than assigning
map.keys().next().value directly, avoiding the inferred any type while
preserving the existing conditional deletion behavior.
Source: Linters/SAST tools
| dek: wrappedDEK, | ||
| dekAt: new Date(), | ||
| }); | ||
| this.dekCache.set(workspaceId, dek); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Bound generated DEKs with the same eviction path.
Backfill calls ensureDEK for every legacy workspace, but this direct set bypasses evictIfNeeded, retaining every raw DEK and allowing memory usage to scale with workspace count.
+ this.evictIfNeeded(this.dekCache);
this.dekCache.set(workspaceId, dek);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.dekCache.set(workspaceId, dek); | |
| this.evictIfNeeded(this.dekCache); | |
| this.dekCache.set(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`
at line 104, Update the DEK caching flow around ensureDEK so generated keys are
inserted through the same bounded-cache path as other entries, invoking
evictIfNeeded after the cache update. Preserve existing DEK generation and
retrieval behavior while ensuring backfill cannot retain unbounded raw DEKs.
1 participant
Summary by CodeRabbit