Conversation
…encryption-in-transit-web-sdk
…encryption-in-transit-web-sdk
… encryption-in-transit-web-sdk
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR introduces Fetch API support and Encryption-In-Transit (AES-GCM-256) capabilities to the CleverTap Web SDK. It adds a new EncryptionInTransit singleton class for payload encryption, extends RequestDispatcher with encrypted request handling and JSONP fallback logic, implements a centralized Logger singleton, and introduces configuration flags to enable/disable these features. Version bumped from 2.4.0 to 2.5.0. Changes
Sequence DiagramssequenceDiagram
participant Client as CleverTap Client
participant RD as RequestDispatcher
participant EIT as EncryptionInTransit
participant Server as Backend Server
Client->>RD: fire(url, params)
RD->>RD: check enableFetchApi && enableEncryptionInTransit
alt Encryption Enabled
RD->>EIT: prepareEncryptedRequest(url)
EIT->>EIT: generateSymmetricKey()
EIT->>EIT: encryptForBackend(payload)
EIT-->>RD: {envelope, key, iv} base64
RD->>Server: fetch(url, encryptedEnvelope)
Server-->>RD: response (402/419 or 200)
else Encryption Disabled or Error
RD->>RD: retryViaJSONP(url)
RD->>Server: inject script tag (JSONP)
Server-->>RD: callback($WZRK_WR)
end
RD->>Client: handleResponse()
sequenceDiagram
participant Client as CleverTap SDK
participant RD as RequestDispatcher
participant LS as localStorage
participant Server as Backend Server
Client->>RD: enableEncryptionInTransit = true
RD->>RD: prepareEncryptedRequest()
alt Encryption Enabled & No Fallback
RD->>Server: fetch(encrypted payload)
alt Server Returns 402/419 (EIT Disabled)
Server-->>RD: 402/419 status
RD->>LS: setItem(CT_EIT_FALLBACK, true)
RD->>RD: retryViaJSONP(url)
RD->>Server: JSONP fallback request
else Success
Server-->>RD: 200 response
RD->>Client: process(decrypted response)
end
else Fallback Active
RD->>LS: check CT_EIT_FALLBACK
RD->>RD: retryViaJSONP(url)
RD->>Server: JSONP request (unencrypted)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.3.13)clevertap.jsThe --json option is unstable/experimental and its output might change between patches/minor releases. ... [truncated 99999795 characters] ... 0;\n H[1] = H[1] + b | 0;\n H[2] = H[2] + c | 0;\n H[3] = H[3] + d | 0;\n H[4] = H[4] + e | 0;\n },\n _doFBiome encountered an unexpected error This is a bug in Biome, not an error in your code, and we would appreciate it if you could report it to https://github.com/biomejs/biome/issues/ along with the following information to help us fixing the issue. When opening the issue, please provide a minimal reproduction, or identify and share the file/code that triggers it. Without a way to reproduce the error, the error can't be fixed: Source Location: crates/biome_console/src/lib.rs:151:14 🔧 ast-grep (0.40.5)clevertap.jsThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 6 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 7
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| } | ||
|
|
||
| this.logger.error('Fetch error:', error); | ||
| }); |
There was a problem hiding this comment.
Missing return statement breaks 420 error retry mechanism
High Severity
The handleFetchResponse function doesn't return the fetch promise, causing the 420 error retry mechanism to fail. When a 420 error occurs, the code at line 9320 does return this.handleFetchResponse(...), but since handleFetchResponse doesn't return anything (the fetch chain has no return statement), undefined is returned instead. The subsequent .then() handler receives undefined, bypasses the rawResponse === null || rawResponse instanceof Promise check, and eventually fails with JSON.parse(undefined). The retry happens in the background but its result is lost.
Additional Locations (1)
|
|
||
| if (config.enableEncryptionInTransit) { | ||
| _classPrivateFieldLooseBase(this, _enableEncryptionInTransit)[_enableEncryptionInTransit] = config.enableEncryptionInTransit; | ||
| RequestDispatcher.enableEncryptionInTransit = config.enableEncryptionInTransit; |
There was a problem hiding this comment.
Missing null check for config parameter causes crash
Medium Severity
The new config.enableFetchApi and config.enableEncryptionInTransit checks at lines 19761 and 19766 access properties directly on config without null safety. Other config property checks in the same init function use optional chaining (e.g., config === null || config === void 0 ? void 0 : config.customId). If a user explicitly passes null as the config parameter, these new lines will throw TypeError: Cannot read property 'enableFetchApi' of null.
| s.async = true; | ||
| document.getElementsByTagName('head')[0].appendChild(s); | ||
| this.logger.debug('req snt -> url: ' + url); | ||
| this.logger.debug('EIT fallback: req snt via JSONP -> url: ' + url); |
There was a problem hiding this comment.
Duplicated JSONP script injection logic
Low Severity
The _retryViaJSONP2 function duplicates the JSONP script injection logic that already exists in _fireRequest2. Both implementations clean up existing ct-jp-cb script elements and create new script tags with identical attributes (type, src, class, rel, async). This redundancy increases maintenance burden and risks inconsistent bug fixes if one location is updated but not the other.
Additional Locations (1)
|
|
||
|
|
||
| const encryptionInTransitInstance = new EncryptionInTransit(); | ||
| window.encryptionInTransitInstance = encryptionInTransitInstance; // Export the singleton instance |
There was a problem hiding this comment.
Encryption instance unnecessarily exposed on window global
Low Severity
The encryptionInTransitInstance is assigned to window.encryptionInTransitInstance but this global reference is never read anywhere in the codebase. The internal encryptionInTransitInstance constant is sufficient for all SDK operations. This appears to be leftover debug code that exposes internal implementation details (including the encryption key stored in this.encryptionKey) to the global scope unnecessarily.
| }).then(rawResponse => { | ||
| // Skip processing if this is a JSONP fallback (null response) or a retry promise | ||
| if (rawResponse === null || rawResponse instanceof Promise) { | ||
| return rawResponse; |
There was a problem hiding this comment.
Dead code: Promise instanceof checks never trigger
Low Severity
The checks rawResponse instanceof Promise at line 9333 and processedResponse instanceof Promise at line 9356 are dead code. In JavaScript Promise chains, when a .then() callback returns a Promise, the chain automatically unwraps it—the next .then() receives the resolved value, not the Promise object itself. These conditions can never evaluate to true, making the associated code paths unreachable.
Additional Locations (1)
| return Promise.resolve({ | ||
| url, | ||
| method: 'GET', | ||
| useFallback: this.isEITFallbackActive() |
There was a problem hiding this comment.
Repeated storage reads for fallback check
Low Severity
In _prepareEncryptedRequest2, isEITFallbackActive() is called three times in quick succession (lines 9588, 9589, and 9596). Each call reads from local storage via StorageManager.read(). The result of this method doesn't change during a single function execution, so calling it once and storing the result in a local variable would be cleaner and more efficient.
Changes
Describe the key changes in this PR with the Jira Issue reference
Changes to Public Facing API if any
Please list the impact on the public facing API if any
How Has This Been Tested?
Describe the testing approach and any relevant configurations (e.g., environment, platform)
Checklist
Link to Deployed SDK
Use these url for testing :
https://static.wizrocket.com/staging/<CURRENT_BRANCH_NAME>/js/clevertap.min.jshttps://static.wizrocket.com/staging/<CURRENT_BRANCH_NAME>/js/sw_webpush.min.jsHow to trigger Automations
Just add a empty commit after all your changes are done in the PR with the command
git commit --allow-empty -m "[run-test] Testing Automation"This will trigger the automation suite
Note
Medium Risk
Modifies the core request dispatch path (Fetch/JSONP selection, URL rewriting, response handling) and introduces crypto + localStorage fallback state, which could impact event delivery when the new flags are enabled.
Overview
Adds feature-flagged “Encryption in Transit” support (v2.5.0). When
enableEncryptionInTransitis enabled,RequestDispatchernow encrypts thedquery parameter using AES-GCM, forces the Fetch transport, and tags requests with an encryption header.Introduces resilience and state for mixed backend support. Fetch responses now handle specific status codes (
402/419to fall back to JSONP for the session viaCT_EIT_FALLBACK,420to retry), and attempt to decrypt response bodies before JSON parsing.Updates SDK version strings to
2.5.0, addsEncryption-in-Transit.md, and updatesCHANGELOG.mdaccordingly.Written by Cursor Bugbot for commit 86e999a. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Release Notes v2.5.0
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.