Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on February 7. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
✅ 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. |
📝 WalkthroughWalkthroughThis PR introduces SDK muting for churned accounts, Encryption-in-Transit (EIT) with JSONP fallback, request dispatcher enhancements, decompression utilities, campaign trigger coordination, and bumps the version to 2.5.1 with comprehensive test coverage and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client SDK
participant Dispatcher as RequestDispatcher
participant Storage as StorageManager
participant Crypto as EncryptionInTransit
participant Server as Server
Client->>Storage: check isMuted()
alt Muted
Storage-->>Client: true → drop request
Client->>Client: log debug, return
else Not Muted
Storage-->>Client: false → proceed
end
Client->>Dispatcher: fireRequest(url)
Dispatcher->>Dispatcher: prepareEncryptedRequest(url)
alt enableEncryptionInTransit
Dispatcher->>Crypto: encryptForBackend(payload)
Crypto-->>Dispatcher: encrypted envelope {itp, itk, itv, id}
Dispatcher->>Dispatcher: set useFetchApi=true
Dispatcher->>Server: handleFetchResponse(encryptedUrl)
else No Encryption
Dispatcher->>Server: retryViaJSONP(url)
end
alt Server Response 402/419
Server-->>Dispatcher: EIT disabled
Dispatcher->>Storage: setEITFallback()
Dispatcher->>Server: retryViaJSONP(originalUrl)
else Server Response 200
Server-->>Dispatcher: {d: response}
alt Encrypted Response
Dispatcher->>Crypto: decryptFromBackend(envelope)
Crypto-->>Dispatcher: decrypted payload
else Plain Response
Dispatcher->>Dispatcher: parse JSON
end
Dispatcher->>Client: apply tracking via $WZRK_WR
end
sequenceDiagram
participant App as Application
participant CleverTap as CleverTap API
participant Dispatcher as RequestDispatcher
participant Storage as StorageManager
App->>CleverTap: init(config)
CleverTap->>Dispatcher: propagate enableEncryptionInTransit
CleverTap->>Dispatcher: propagate enableFetchApi
CleverTap->>Dispatcher: clearEITFallback()
CleverTap->>Storage: clear CT_EIT_FALLBACK
App->>CleverTap: enableEncryptionInTransit = true
CleverTap->>Dispatcher: setter updates Dispatcher.enableEncryptionInTransit
App->>CleverTap: isMuted()
CleverTap->>Storage: isMuted()
Storage-->>CleverTap: boolean result
CleverTap-->>App: current mute state
App->>CleverTap: getMuteExpiry()
CleverTap->>Storage: getMuteExpiry()
Storage-->>CleverTap: epoch timestamp or null
CleverTap-->>App: expiry timestamp or null
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@clevertap.js`:
- Around line 226-228: Move the "SDK Muting - Progressive muting for churned
customers" comment from the ISOLATE_COOKIE declaration to the MUTE_EXPIRY_KEY
declaration so the muting description aligns with the muting constant; update
comments around the ISOLATE_COOKIE and MUTE_EXPIRY_KEY symbols accordingly to
reflect that ISOLATE_COOKIE is for sub-domain isolation and MUTE_EXPIRY_KEY is
for SDK muting.
- Around line 7960-7974: The setMuteExpiry method on CleverTapAPI is not being
exposed on the api object used to populate window.$WZRK_WR, so server calls
cannot reach it; add an assignment to expose it (e.g., assign api.setMuteExpiry
= this.setMuteExpiry) alongside the existing api.getMuteExpiry and api.isMuted
bindings in the api setup where the $WZRK_WR methods are assembled so the server
can call CleverTapAPI.prototype.setMuteExpiry; ensure the binding references the
same instance (this) as the other methods.
- Around line 7574-7580: clearMuteExpiry currently only removes MUTE_EXPIRY_KEY
from localStorage and globalCache, leaving a cookie set by saveToLSorCookie when
localStorage is unavailable; update clearMuteExpiry to also remove the cookie
using the same domain used by saveToLSorCookie (window.location.hostname) rather
than $ct.broadDomain, ensuring you reference MUTE_EXPIRY_KEY and remove the
cookie before deleting $ct.globalCache[MUTE_EXPIRY_KEY]; keep the
StorageManager.remove call for localStorage support intact.
In `@src/util/storage.js`:
- Around line 323-327: getMuteExpiry currently returns whatever
StorageManager.readFromLSorCookie(MUTE_EXPIRY_KEY) provides (possibly an expired
timestamp or string); change it so it reads the stored value, coerces/parses it
to a numeric epoch ms, and returns null if missing or if parsed value <=
Date.now() (otherwise return the numeric timestamp). Update getMuteExpiry to use
MUTE_EXPIRY_KEY and StorageManager.readFromLSorCookie, guard against non-numeric
values, and ensure callers receive null for expired or absent mutes.
- Around line 333-337: clearMuteExpiry currently only removes the mute key from
localStorage when StorageManager._isLocalStorageSupported() is true and always
deletes $ct.globalCache[MUTE_EXPIRY_KEY], but it never clears the cookie in
cookie-only environments; update clearMuteExpiry to, in the else-path where
localStorage is unsupported, also remove the cookie by invoking the appropriate
cookie removal API on StorageManager (e.g.,
StorageManager.removeCookie(MUTE_EXPIRY_KEY) or the project’s cookie deletion
helper), and then still delete $ct.globalCache[MUTE_EXPIRY_KEY] so the SDK state
is fully cleared.
🧹 Nitpick comments (4)
src/util/constants.js (1)
79-81: Prefer a single source of truth for the mute key.Now that
MUTE_EXPIRY_KEYis exported, consider replacing the localconst MUTE_EXPIRY_KEYinsrc/modules/clevertap.js(around Line 227) with an import fromsrc/util/constants.jsto avoid drift.src/util/requestDispatcher.js (1)
17-22: Duplicate mute check may be redundant.This mute check in
#fireRequestduplicates the check already performed inRequestManager.saveAndFireRequest(src/modules/request.js, lines 142-147). SincesaveAndFireRequestcallsRequestDispatcher.fireRequest, requests flowing through that path are checked twice.Consider whether this is intentional defense-in-depth (e.g., for direct
fireRequestcalls that bypasssaveAndFireRequest) or if one check can be removed. Direct callers likeregisterToken,unregisterTokenForGuid, andprocessBackupEventsdo callfireRequestdirectly, so keeping this check here ensures coverage for those paths.src/modules/request.js (1)
142-147: LGTM - Mute check correctly prevents backup creation for churned accounts.The mute check at this entry point correctly drops requests before backup creation, which differs intentionally from offline mode behavior. This ensures churned accounts don't accumulate events that would be sent later.
Minor observation: The log message here is simpler than the one in
requestDispatcher.js(which includes the expiry timestamp). Consider adding the expiry time for consistency:if (isMuted()) { - this.#logger.debug('Request dropped - SDK is muted') + const { getMuteExpiry } = require('../util/storage') + this.#logger.debug('Request dropped - SDK is muted until ' + new Date(getMuteExpiry()).toISOString()) return }src/modules/api.js (1)
158-163: Consider logging invalid input for debugging.The method silently ignores invalid input (non-number or non-positive values). This could make debugging harder if the server sends malformed data.
🔧 Proposed enhancement for better observability
setMuteExpiry (muteExpiryMs) { if (typeof muteExpiryMs === 'number' && muteExpiryMs > 0) { StorageManager.saveToLSorCookie(MUTE_EXPIRY_KEY, muteExpiryMs) this.#logger.info(`SDK muted until: ${new Date(muteExpiryMs).toISOString()}`) + } else { + this.#logger.debug(`Invalid muteExpiryMs value ignored: ${muteExpiryMs}`) } }
| const clearMuteExpiry = () => { | ||
| if (StorageManager._isLocalStorageSupported()) { | ||
| StorageManager.remove(MUTE_EXPIRY_KEY); | ||
| } | ||
|
|
||
| delete $ct.globalCache[MUTE_EXPIRY_KEY]; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the setMuteExpiry function to see how it stores the mute expiry
rg -n -B2 -A10 'const setMuteExpiry' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 53
🏁 Script executed:
# Find StorageManager.remove implementation
rg -n -B2 -A8 'remove\s*[:=].*function|remove\s*\(.*\)\s*{' clevertap.js | head -40Repository: CleverTap/clevertap-web-sdk
Length of output: 220
🏁 Script executed:
# Find StorageManager.removeCookie implementation
rg -n -B2 -A8 'removeCookie' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 3945
🏁 Script executed:
# Find saveToLSorCookie implementation
rg -n -B2 -A15 'saveToLSorCookie' clevertap.js | head -60Repository: CleverTap/clevertap-web-sdk
Length of output: 1984
🏁 Script executed:
# Find $ct.broadDomain initialization
rg -n 'broadDomain\s*[:=]' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 286
🏁 Script executed:
# Find setMuteExpiry function and MUTE_EXPIRY_KEY usage
rg -n -B2 -A10 'setMuteExpiry|MUTE_EXPIRY_KEY' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 2395
🏁 Script executed:
# Look at the readFromLSorCookie to understand the dual-storage retrieval
rg -n -B2 -A15 'readFromLSorCookie' clevertap.js | head -80Repository: CleverTap/clevertap-web-sdk
Length of output: 2670
🏁 Script executed:
# Find StorageManager.remove implementation
rg -n -B2 -A8 'static remove' clevertap.js | grep -A10 'remove\('Repository: CleverTap/clevertap-web-sdk
Length of output: 94
🏁 Script executed:
# Also check what domain is used when saveToLSorCookie saves to cookies
rg -n -B5 -A10 'createCookie.*property.*encodeURIComponent' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 735
🏁 Script executed:
# Find the remove method implementation
rg -n -B2 -A8 'static remove\s*\(' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 318
🏁 Script executed:
# Also check what the save() method does
rg -n -B2 -A8 'static save\s*\(' clevertap.js | head -30Repository: CleverTap/clevertap-web-sdk
Length of output: 417
🏁 Script executed:
# Check if there are other examples of clearing cookies saved via saveToLSorCookie
rg -n -B5 -A5 'saveToLSorCookie' clevertap.js | grep -A10 -B10 'removeCookie\|remove'Repository: CleverTap/clevertap-web-sdk
Length of output: 2223
🏁 Script executed:
# Check createCookie to understand domain handling
rg -n -B2 -A20 'static createCookie' clevertap.js | head -50Repository: CleverTap/clevertap-web-sdk
Length of output: 776
Mute state not cleared from cookies when localStorage is unavailable.
setMuteExpiry uses saveToLSorCookie which falls back to cookies (domain: window.location.hostname) when localStorage is unavailable. However, clearMuteExpiry only removes from localStorage via StorageManager.remove(), leaving the mute state persisted in cookies. This means accounts cannot be "unmuted" on devices without localStorage support.
The proposed fix should use window.location.hostname as the domain (matching where the cookie is stored), not $ct.broadDomain:
Corrected fix
const clearMuteExpiry = () => {
if (StorageManager._isLocalStorageSupported()) {
StorageManager.remove(MUTE_EXPIRY_KEY);
+ } else {
+ StorageManager.removeCookie(MUTE_EXPIRY_KEY, window.location.hostname);
}
delete $ct.globalCache[MUTE_EXPIRY_KEY];
};🤖 Prompt for AI Agents
In `@clevertap.js` around lines 7574 - 7580, clearMuteExpiry currently only
removes MUTE_EXPIRY_KEY from localStorage and globalCache, leaving a cookie set
by saveToLSorCookie when localStorage is unavailable; update clearMuteExpiry to
also remove the cookie using the same domain used by saveToLSorCookie
(window.location.hostname) rather than $ct.broadDomain, ensuring you reference
MUTE_EXPIRY_KEY and remove the cookie before deleting
$ct.globalCache[MUTE_EXPIRY_KEY]; keep the StorageManager.remove call for
localStorage support intact.
| /** | ||
| * Set SDK mute expiry timestamp (for churned accounts) | ||
| * Called by server when account is blocked/muted | ||
| * Server calls: $WZRK_WR.setMuteExpiry(1737100800000) | ||
| * @param {number} muteExpiryMs - Epoch timestamp in ms until which SDK should be muted | ||
| */ | ||
|
|
||
|
|
||
| setMuteExpiry(muteExpiryMs) { | ||
| if (typeof muteExpiryMs === 'number' && muteExpiryMs > 0) { | ||
| StorageManager.saveToLSorCookie(MUTE_EXPIRY_KEY, muteExpiryMs); | ||
|
|
||
| _classPrivateFieldLooseBase(this, _logger$a)[_logger$a].info("SDK muted until: ".concat(new Date(muteExpiryMs).toISOString())); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for setMuteExpiry definitions and usages
echo "=== Searching for setMuteExpiry definitions ==="
rg -n 'setMuteExpiry' clevertap.js | head -20
echo ""
echo "=== Searching for $WZRK_WR assignments and setup ==="
rg -n '\$WZRK_WR' clevertap.js | head -30
echo ""
echo "=== Looking for window.$WZRK_WR setup ==="
rg -n 'window\.\$WZRK_WR' clevertap.js | head -20Repository: CleverTap/clevertap-web-sdk
Length of output: 674
🏁 Script executed:
#!/bin/bash
# Check what 'api' is and how setMuteExpiry class is related
echo "=== Looking for the class containing setMuteExpiry ==="
rg -n -B30 'setMuteExpiry(muteExpiryMs)' clevertap.js | head -60
echo ""
echo "=== Checking the api object definition and what it contains ==="
rg -n 'const api' clevertap.js
rg -n 'let api' clevertap.js
rg -n 'var api' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 261
🏁 Script executed:
#!/bin/bash
# Read the context around the setMuteExpiry method definition
echo "=== Context around setMuteExpiry method (lines 7950-7980) ==="
sed -n '7950,7980p' clevertap.js
echo ""
echo "=== Find the class that contains this method ==="
sed -n '7900,7970p' clevertap.js | grep -n 'class\|setMuteExpiry'
echo ""
echo "=== Check what api object is assigned to $WZRK_WR ==="
sed -n '18300,18320p' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 2972
🏁 Script executed:
#!/bin/bash
# Search for where methods are assigned to the api object
echo "=== Find all api.* assignments ==="
rg -n 'api\.' clevertap.js | grep -E '(api\.\w+\s*=|getMuteExpiry|isMuted|setMuteExpiry)' | head -30
echo ""
echo "=== Look at the full context of api object setup (lines 18200-18330) ==="
sed -n '18200,18330p' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 5241
🏁 Script executed:
#!/bin/bash
# Search for setMuteExpiry assignment to api object
echo "=== Search for setMuteExpiry being assigned to api ==="
rg -n 'api\.setMuteExpiry' clevertap.js
echo ""
echo "=== Check if setMuteExpiry is bound to the api object anywhere ==="
rg -n 'setMuteExpiry' clevertap.js -A3 -B3 | head -50
echo ""
echo "=== Look at complete api assignments section (18300-18320) ==="
sed -n '18300,18320p' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 1460
🏁 Script executed:
#!/bin/bash
# Find which class contains the setMuteExpiry method
echo "=== Find the class definition containing setMuteExpiry ==="
sed -n '7800,7970p' clevertap.js | grep -n 'class\|setMuteExpiry' | tail -20
echo ""
echo "=== Look for where 'this' refers in context, find class name ==="
sed -n '7700,7800p' clevertap.js | grep -n '^class\|^ *class'
echo ""
echo "=== Check the structure: is setMuteExpiry on the main class? ==="
sed -n '7960,7980p' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 3164
🏁 Script executed:
#!/bin/bash
# Confirm that setMuteExpiry is not in the api assignments
echo "=== Complete api assignments for muting-related functions ==="
sed -n '18299,18315p' clevertap.js
echo ""
echo "=== Verify setMuteExpiry is a method on CleverTapAPI ==="
sed -n '7968,7973p' clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 924
setMuteExpiry() is not exposed to $WZRK_WR and cannot be called from the server as documented.
The method is defined on the CleverTapAPI class but is missing from the api object assignments. Only getMuteExpiry and isMuted are exposed to window.$WZRK_WR (lines 18306-18310). The setMuteExpiry method must be assigned to the api object for server-side calls to work, e.g.:
api.setMuteExpiry = this.setMuteExpiry;
This assignment needs to be added in the api setup section around line 18310.
🤖 Prompt for AI Agents
In `@clevertap.js` around lines 7960 - 7974, The setMuteExpiry method on
CleverTapAPI is not being exposed on the api object used to populate
window.$WZRK_WR, so server calls cannot reach it; add an assignment to expose it
(e.g., assign api.setMuteExpiry = this.setMuteExpiry) alongside the existing
api.getMuteExpiry and api.isMuted bindings in the api setup where the $WZRK_WR
methods are assembled so the server can call
CleverTapAPI.prototype.setMuteExpiry; ensure the binding references the same
instance (this) as the other methods.
| * Get the mute expiry timestamp | ||
| * @returns {number|null} epoch timestamp in ms, or null if not muted | ||
| */ | ||
| export const getMuteExpiry = () => { | ||
| return StorageManager.readFromLSorCookie(MUTE_EXPIRY_KEY) || null |
There was a problem hiding this comment.
Align getMuteExpiry with its JSDoc (“null if not muted”).
Right now an expired timestamp still gets returned. Either update the JSDoc or guard against expired values so callers don’t treat an expired mute as active.
✅ Suggested fix (return null when expired)
export const getMuteExpiry = () => {
- return StorageManager.readFromLSorCookie(MUTE_EXPIRY_KEY) || null
+ const muteExpiry = StorageManager.readFromLSorCookie(MUTE_EXPIRY_KEY)
+ if (!muteExpiry || muteExpiry <= 0) {
+ return null
+ }
+ return Date.now() < muteExpiry ? muteExpiry : null
}📝 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.
| * Get the mute expiry timestamp | |
| * @returns {number|null} epoch timestamp in ms, or null if not muted | |
| */ | |
| export const getMuteExpiry = () => { | |
| return StorageManager.readFromLSorCookie(MUTE_EXPIRY_KEY) || null | |
| * Get the mute expiry timestamp | |
| * `@returns` {number|null} epoch timestamp in ms, or null if not muted | |
| */ | |
| export const getMuteExpiry = () => { | |
| const muteExpiry = StorageManager.readFromLSorCookie(MUTE_EXPIRY_KEY) | |
| if (!muteExpiry || muteExpiry <= 0) { | |
| return null | |
| } | |
| return Date.now() < muteExpiry ? muteExpiry : null | |
| } |
🤖 Prompt for AI Agents
In `@src/util/storage.js` around lines 323 - 327, getMuteExpiry currently returns
whatever StorageManager.readFromLSorCookie(MUTE_EXPIRY_KEY) provides (possibly
an expired timestamp or string); change it so it reads the stored value,
coerces/parses it to a numeric epoch ms, and returns null if missing or if
parsed value <= Date.now() (otherwise return the numeric timestamp). Update
getMuteExpiry to use MUTE_EXPIRY_KEY and StorageManager.readFromLSorCookie,
guard against non-numeric values, and ensure callers receive null for expired or
absent mutes.
src/util/storage.js
Outdated
| export const clearMuteExpiry = () => { | ||
| if (StorageManager._isLocalStorageSupported()) { | ||
| StorageManager.remove(MUTE_EXPIRY_KEY) | ||
| } | ||
| delete $ct.globalCache[MUTE_EXPIRY_KEY] |
There was a problem hiding this comment.
Clear mute state for cookie-only environments.
If localStorage is unavailable, clearMuteExpiry won’t remove the cookie, so the SDK may remain muted. Please clear the cookie in that path.
🔧 Proposed fix
export const clearMuteExpiry = () => {
if (StorageManager._isLocalStorageSupported()) {
StorageManager.remove(MUTE_EXPIRY_KEY)
+ } else {
+ StorageManager.removeCookie(MUTE_EXPIRY_KEY, window.location.hostname)
}
delete $ct.globalCache[MUTE_EXPIRY_KEY]
}📝 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.
| export const clearMuteExpiry = () => { | |
| if (StorageManager._isLocalStorageSupported()) { | |
| StorageManager.remove(MUTE_EXPIRY_KEY) | |
| } | |
| delete $ct.globalCache[MUTE_EXPIRY_KEY] | |
| export const clearMuteExpiry = () => { | |
| if (StorageManager._isLocalStorageSupported()) { | |
| StorageManager.remove(MUTE_EXPIRY_KEY) | |
| } else { | |
| StorageManager.removeCookie(MUTE_EXPIRY_KEY, window.location.hostname) | |
| } | |
| delete $ct.globalCache[MUTE_EXPIRY_KEY] | |
| } |
🤖 Prompt for AI Agents
In `@src/util/storage.js` around lines 333 - 337, clearMuteExpiry currently only
removes the mute key from localStorage when
StorageManager._isLocalStorageSupported() is true and always deletes
$ct.globalCache[MUTE_EXPIRY_KEY], but it never clears the cookie in cookie-only
environments; update clearMuteExpiry to, in the else-path where localStorage is
unsupported, also remove the cookie by invoking the appropriate cookie removal
API on StorageManager (e.g., StorageManager.removeCookie(MUTE_EXPIRY_KEY) or the
project’s cookie deletion helper), and then still delete
$ct.globalCache[MUTE_EXPIRY_KEY] so the SDK state is fully cleared.
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In `@clevertap.js`:
- Around line 9306-9311: The static method setEITFallback uses this.logger which
is confusing in a static context; change it to reference the static logger
explicitly (RequestDispatcher.logger) so the log call always uses the
class-level logger. In the setEITFallback method (alongside StorageManager and
CT_EIT_FALLBACK references), replace this.logger.debug(...) with
RequestDispatcher.logger.debug(...) to avoid relying on dynamic this binding and
ensure clarity and correctness.
- Around line 228-230: The comment for MUTE_EXPIRY_KEY is incorrect; update the
comment next to the constant MUTE_EXPIRY_KEY to describe that it is the flag key
used for SDK muting/progressive muting (for churned customers) rather than
"Encryption in Transit JSONP fallback", and if needed move or add the EIT
fallback description to the CT_EIT_FALLBACK constant so CT_EIT_FALLBACK is
documented as the Encryption in Transit JSONP fallback key; update only the
inline comments for MUTE_EXPIRY_KEY and CT_EIT_FALLBACK to reflect these correct
responsibilities.
- Around line 9267-9268: The code exposes the EncryptionInTransit instance
globally via window.encryptionInTransitInstance which leaks keys/methods; remove
the global assignment and instead export or return the instance from the module
(use the EncryptionInTransit class and encryptionInTransitInstance symbol) so
consumers import it explicitly, or if you must expose it for debugging only,
gate the window assignment behind a non-production check (e.g., NODE_ENV or a
debug flag) so production builds never set window.encryptionInTransitInstance.
In `@Encryption-in-Transit.md`:
- Around line 114-127: The documentation snippet for decryptFromBackend is out
of sync: remove extraction of itk from the envelope and update the code to parse
only itp and itv, then use the stored singleton key (this.encryptionKey /
instance encryptionKey) instead of the envelope key; specifically, change the
JSON.parse destructuring to only { itp, itv }, decode itp and itv as before, and
call crypto.subtle.importKey/import with the stored key (this.encryptionKey) and
AES-GCM alg before decrypting the payload in decryptFromBackend.
In `@src/util/constants.js`:
- Around line 75-84: Remove the redundant local constant declarations for
ISOLATE_COOKIE, MUTE_EXPIRY_KEY, and CT_EIT_FALLBACK in src/clevertap.js (they
currently shadow the imported ISOLATE_COOKIE and duplicate the other two);
instead, add MUTE_EXPIRY_KEY and CT_EIT_FALLBACK to the existing import from
src/util/constants so all three values come from the shared exports, and verify
all usages in clevertap.js reference the imported symbols (no local consts
remain).
In `@src/util/encoder.js`:
- Around line 387-388: The decompress function returns mixed types for
error/edge cases (empty string in most paths but null on invalid dictionary
lookup), which breaks callers like decompressFromBase64; update decompress so
every error/edge return consistently returns an empty string (''), specifically
replace the null return in the invalid dictionary lookup path inside decompress
with '', and verify the other early returns (empty input, end-of-stream marker,
index out-of-range) remain ''. Also scan any internal branches in decompress
that currently return null and make them return '' so callers don't need to
handle null.
In `@src/util/requestDispatcher.js`:
- Around line 324-328: The .then((rawResponse) => { ... }) callback in
requestDispatcher.js contains a redundant rawResponse instanceof Promise check
that masks a broken promise chain; fix the earlier recursive retry call (the
retry path that re-invokes the dispatcher) so it returns/propagates its Promise
instead of breaking the chain, then remove the instanceof Promise checks (and
the corresponding duplicate at the later .then) so the .then always receives a
resolved value or null for JSONP fallback. Ensure the retry function/method (the
recursive invocation used for retries) returns the Promise it creates so
downstream code in the .then handlers no longer needs to guard for Promise
instances.
- Around line 364-383: The code calls window.$WZRK_WR.tr, .s and .enableWebPush
directly which can throw if the SDK object is missing; update the block that
reads jsonResponse (the references to tr, meta, wpe and calls to
window.$WZRK_WR.tr, window.$WZRK_WR.s, window.$WZRK_WR.enableWebPush) to first
guard that window.$WZRK_WR is defined and that each method is a function before
invoking it (e.g., if (window.$WZRK_WR && typeof window.$WZRK_WR.tr ===
'function') window.$WZRK_WR.tr(tr); and similar guards for .s and
.enableWebPush), keeping the existing parameter handling (including optOut)
unchanged.
- Around line 309-318: The retry logic in handleFetchResponse currently returns
a Promise from the recursive call but the function itself doesn't return a
meaningful value and retries the same encryptedUrl (which won't help); update
handleFetchResponse to return the Promise/result of its operations (so callers
get the retry outcome) and change the retry branch to re-encrypt the original
payload before retrying (e.g., produce a newEncryptedUrl from originalUrl via
the module's encryption helper such as this.encryptPayload(originalUrl) or
similar) then return this.handleFetchResponse(newEncryptedUrl, originalUrl,
retryCount + 1); keep logging via this.logger.debug/ error and cap retries at 3
as before.
- Around line 33-38: The isEITFallbackActive method uses
StorageManager.read(CT_EIT_FALLBACK) === true which can fail when
StorageManager.read returns the original string on JSON.parse errors; fix by
reading into a variable and normalizing/coercing the result to a boolean before
returning (e.g., handle boolean true or the string "true" or explicitly try
JSON.parse with a safe fallback). Update static isEITFallbackActive in
requestDispatcher.js and reference StorageManager.read and CT_EIT_FALLBACK; keep
the prior local storage support guard StorageManager._isLocalStorageSupported()
unchanged.
- Line 289: The fetch(encryptedUrl, fetchOptions) call currently has no timeout;
wrap it with an AbortController: create a controller, add its signal to
fetchOptions (signal: controller.signal), start a setTimeout (e.g., 10000 ms or
a configurable constant) that calls controller.abort() to cancel the request,
clear that timeout on successful resolution or rejection, and ensure you handle
the abort error path (e.g., treat as timeout) in the same function where
fetch(encryptedUrl, fetchOptions) is invoked so callers receive a clear timeout
error instead of hanging.
In `@src/util/security/encryptionInTransit.js`:
- Around line 19-21: The toB64 function uses String.fromCharCode(...u8), which
breaks on large Uint8Array inputs; update the toB64 implementation to convert
the Uint8Array to a string in chunks (e.g., loop over u8 in fixed-size slices
and call String.fromCharCode on each slice) and then call btoa on the
concatenated string; ensure to handle Uint8Array inputs and choose a chunk size
that avoids exceeding call stack (e.g., 32KB-ish) and preserve existing return
behavior.
- Around line 87-93: The envelope currently includes the raw symmetric key (itk
= this.toB64(this.encryptionKey)), which defeats confidentiality; instead stop
serializing/transmitting this.encryptionKey directly and implement asymmetric
key-wrapping or key-exchange: before building envelope, encrypt/wrap
this.encryptionKey with the recipient's public key (or derive a shared key via
ECDH) and set itk to the wrapped-key blob (base64) or omit itk entirely if using
an established session key; update the corresponding decryption path that
consumes the envelope (the code that reads itp/itk and the decryption routine
referenced near the decryption logic) to unwrap the symmetric key with the
matching private key or session secret and then decrypt itp with the unwrapped
key; also add a short comment documenting the chosen threat model (TLS-only vs.
application-layer key-wrapping) so backend/integration expectations are
explicit.
In `@test-encryption.js`:
- Around line 1-2: The current import uses named imports encryptForBackend and
decryptFromBackend but the module only exposes a default export; change the
import to default (import encryptionInTransitInstance from
'./src/util/security/encryptionInTransit.js') and update call sites to use
encryptionInTransitInstance.encryptForBackend(...) and
encryptionInTransitInstance.decryptFromBackend(...), ensuring all references to
encryptForBackend and decryptFromBackend are replaced accordingly.
🧹 Nitpick comments (13)
CHANGELOG.md (1)
4-8: Inconsistent date format between changelog entries.Line 4 uses "30 Jan 2026" while Line 7 uses "29th Jan 2026" (with ordinal suffix). Consider using a consistent date format throughout the changelog for better readability.
Suggested fix
## [2.5.1] 30 Jan 2026 - Fixed the campaign delivery triggers logic -## [2.5.0] 29th Jan 2026 +## [2.5.0] 29 Jan 2026 - Fetch api and encryption supporttest/unit/main.spec.js (2)
26-29:globalCachemock is missinggcookieproperty.According to the actual
$ct.globalCachedefinition (in clevertap.js), the object includesgcookie: null. While tests may not need it currently, omitting it could cause issues if future tests or the code under test reference$ct.globalCache.gcookie.Suggested fix
$ct.globalCache = { + gcookie: null, REQ_N: 0, RESP_N: 0 }
7-15: Redundant mock reassignment inbeforeEach.The
RequestDispatchermethods are already mocked at the module level (lines 7-15). Reassigning them inbeforeEach(lines 20-21) is unnecessary unless you need to reset call counts, which could be achieved withjest.clearAllMocks()instead.Suggested simplification
describe('main.js', function () { beforeEach(() => { - // Mock RequestDispatcher methods to prevent any unexpected calls - RequestDispatcher.handleFetchResponse = jest.fn().mockResolvedValue() - RequestDispatcher.fireRequest = jest.fn() + jest.clearAllMocks() // Mock $ct object with default values to prevent undefined errorsAlso applies to: 18-22
src/util/encoder.js (1)
460-463: Off-by-one check in while loop guard.Line 461 uses
data.index > compressed.lengthbut the index is used to read fromcompressedwithcompressed.charCodeAt(data.index++). This check should use>=to avoid attempting to read at an invalid index before the increment occurs.However, looking at the read pattern (e.g., line 473), the index is only incremented after a successful read, and the charCodeAt call handles out-of-bounds by returning NaN/0. The current logic appears safe but could be clearer.
clevertap.js (2)
9340-9467: Static methodhandleFetchResponseusesthisreferences throughout.This static method uses
this.logger,this.setEITFallback(),this.enableEncryptionInTransit, etc. While these resolve to static properties/methods on the class, it's inconsistent and confusing. Consider usingRequestDispatcher.prefix explicitly for all static references.
9646-9661: Static method reference should use class name.Line 9648 and 9656 use
this.isEITFallbackActive()butisEITFallbackActiveis a static method. UseRequestDispatcher.isEITFallbackActive()for clarity.Proposed fix
var _prepareEncryptedRequest2 = function _prepareEncryptedRequest2(url) { // Check if encryption is disabled or fallback is active - if (!this.enableEncryptionInTransit || this.isEITFallbackActive()) { - if (this.isEITFallbackActive() && this.enableEncryptionInTransit) { + if (!this.enableEncryptionInTransit || RequestDispatcher.isEITFallbackActive()) { + if (RequestDispatcher.isEITFallbackActive() && this.enableEncryptionInTransit) { this.logger.debug('EIT fallback active - bypassing encryption for this session'); } return Promise.resolve({ url, method: 'GET', - useFallback: this.isEITFallbackActive() + useFallback: RequestDispatcher.isEITFallbackActive() }); }test-encryption.js (1)
1-69: Consider whether this test file should be included in the repository.This appears to be a development/debugging script rather than a formal test:
- Uses
console.logfor output instead of test assertions- No process exit codes for CI integration
- Located at repository root rather than in
test/directoryIf intended for CI, consider:
- Moving to
test/integration/or similar- Adding proper exit codes (
process.exit(1)on failure)- Converting to a formal test using Jest
If it's only for local debugging, consider adding it to
.gitignoreor marking it clearly as a development-only utility.test/unit/util/requestDispatcher.spec.js (1)
175-197: Consider consolidating duplicative test assertions.The tests in "fallback flag interaction with encryption setting" (lines 175-197) appear to duplicate assertions already covered earlier in the suite:
isEITFallbackActive returns correct value based on storage read(lines 176-184) duplicates tests at lines 116-129setEITFallback saves the correct key and value(lines 186-190) duplicates the test at lines 133-139clearEITFallback removes the correct key(lines 192-196) duplicates the test at lines 149-152Consider removing these duplicates to improve test maintainability, or if they serve a distinct purpose (e.g., testing interaction with encryption settings), add comments clarifying their intent.
Encryption-in-Transit.md (1)
186-196: Minor: Repetitive sentence structure.Lines 188-194 begin six consecutive items with "SDK" or "User". Consider varying the sentence structure for better readability:
- Error logged –
console.error("Encryption in Transit is disabled on server side").- Immediate retry – the same request is resent using JSONP with no encryption.
- Fallback flag set in local storage (
CT_EIT_FALLBACK = true).This is a minor style suggestion for documentation polish.
src/util/security/encryptionInTransit.js (2)
109-155: Inconsistent async patterns and redundant error handling.The method is declared
asyncbut usesreturn Promise.reject()(lines 116, 121, 153) instead ofthrow. This works but is inconsistent with idiomatic async/await. Also, the outer try-catch and inner.catch()create redundant error paths.Additionally,
TextDecoderis instantiated inline (line 146) whileTextEncoderis cached in the constructor - minor inconsistency.♻️ Suggested refactor using consistent async/await
- async decryptFromBackend (envelope) { - try { - // Parse the envelope from backend - const parsedEnvelope = JSON.parse(envelope) - const { itp, itv } = parsedEnvelope - - if (!itp || !itv) { - return Promise.reject(new Error('Decryption failed: Invalid envelope format')) - } - - // Check if encryption key exists - if (!this.encryptionKey) { - return Promise.reject(new Error('Decryption failed: No encryption key available')) - } + async decryptFromBackend (envelope) { + const parsedEnvelope = JSON.parse(envelope) + const { itp, itv } = parsedEnvelope + + if (!itp || !itv) { + throw new Error('Decryption failed: Invalid envelope format') + } + + if (!this.encryptionKey) { + throw new Error('Decryption failed: No encryption key available') + }And use
awaitfor the crypto operations instead of.then()chains.
158-163: Consider removing global window attachment.Attaching the encryption instance to
window.encryptionInTransitInstanceexposes it to any script on the page, including potentially malicious third-party scripts. They could inspectencryptionKeyor manipulate the instance.If this is needed for debugging, consider gating it behind a debug flag.
♻️ Proposed fix
// Create and export singleton instance const encryptionInTransitInstance = new EncryptionInTransit() -window.encryptionInTransitInstance = encryptionInTransitInstance +// Only expose for debugging in non-production builds +if (process.env.NODE_ENV === 'development') { + window.encryptionInTransitInstance = encryptionInTransitInstance +} // Export the singleton instance export default encryptionInTransitInstancesrc/util/requestDispatcher.js (2)
98-99: Avoid mutating global state as side effect.Setting
this.enableFetchApi = trueinside#prepareEncryptedRequestis a hidden side effect that changes class-level state. This makes the code harder to reason about and could cause unexpected behavior if encryption is later disabled.Consider either:
- Making this explicit in the calling code
- Documenting this behavior prominently
310-310: Extract magic number to a named constant.The retry limit
3should be a named constant for clarity and maintainability.♻️ Proposed fix
+import { ARP_COOKIE, MAX_TRIES, OPTOUT_COOKIE_ENDSWITH, USEIP_KEY, MAX_DELAY_FREQUENCY, PUSH_DELAY_MS, WZRK_FETCH, CT_EIT_FALLBACK, MAX_DECRYPT_RETRIES } from './constants' // In constants.js add: +export const MAX_DECRYPT_RETRIES = 3 // Then in handleFetchResponse: - if (retryCount < 3) { + if (retryCount < MAX_DECRYPT_RETRIES) {
| const MUTE_EXPIRY_KEY = 'WZRK_MUTE_EXPIRY'; // Flag key for Encryption in Transit JSONP fallback (session-level) | ||
|
|
||
| const CT_EIT_FALLBACK = 'CT_EIT_FALLBACK'; |
There was a problem hiding this comment.
Comment on MUTE_EXPIRY_KEY is incorrect.
The comment says "Flag key for Encryption in Transit JSONP fallback" but MUTE_EXPIRY_KEY is for SDK muting (progressive muting for churned customers). CT_EIT_FALLBACK on line 230 is the actual EIT fallback key.
Proposed fix
- const MUTE_EXPIRY_KEY = 'WZRK_MUTE_EXPIRY'; // Flag key for Encryption in Transit JSONP fallback (session-level)
-
- const CT_EIT_FALLBACK = 'CT_EIT_FALLBACK';
+ // SDK Muting - Progressive muting for churned customers
+ const MUTE_EXPIRY_KEY = 'WZRK_MUTE_EXPIRY';
+
+ // Flag key for Encryption in Transit JSONP fallback (session-level)
+ const CT_EIT_FALLBACK = 'CT_EIT_FALLBACK';🤖 Prompt for AI Agents
In `@clevertap.js` around lines 228 - 230, The comment for MUTE_EXPIRY_KEY is
incorrect; update the comment next to the constant MUTE_EXPIRY_KEY to describe
that it is the flag key used for SDK muting/progressive muting (for churned
customers) rather than "Encryption in Transit JSONP fallback", and if needed
move or add the EIT fallback description to the CT_EIT_FALLBACK constant so
CT_EIT_FALLBACK is documented as the Encryption in Transit JSONP fallback key;
update only the inline comments for MUTE_EXPIRY_KEY and CT_EIT_FALLBACK to
reflect these correct responsibilities.
| const encryptionInTransitInstance = new EncryptionInTransit(); | ||
| window.encryptionInTransitInstance = encryptionInTransitInstance; // Export the singleton instance |
There was a problem hiding this comment.
Exposing encryption instance on window is a security risk.
window.encryptionInTransitInstance exposes the encryption key and methods globally, allowing any script on the page to access or tamper with the encryption state. This undermines the security benefits of encryption-in-transit.
Consider removing this global exposure or, if needed for debugging, only expose it in non-production builds.
Proposed fix
const encryptionInTransitInstance = new EncryptionInTransit();
-window.encryptionInTransitInstance = encryptionInTransitInstance; // Export the singleton instance📝 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 encryptionInTransitInstance = new EncryptionInTransit(); | |
| window.encryptionInTransitInstance = encryptionInTransitInstance; // Export the singleton instance | |
| const encryptionInTransitInstance = new EncryptionInTransit(); |
🤖 Prompt for AI Agents
In `@clevertap.js` around lines 9267 - 9268, The code exposes the
EncryptionInTransit instance globally via window.encryptionInTransitInstance
which leaks keys/methods; remove the global assignment and instead export or
return the instance from the module (use the EncryptionInTransit class and
encryptionInTransitInstance symbol) so consumers import it explicitly, or if you
must expose it for debugging only, gate the window assignment behind a
non-production check (e.g., NODE_ENV or a debug flag) so production builds never
set window.encryptionInTransitInstance.
| static setEITFallback() { | ||
| if (StorageManager._isLocalStorageSupported()) { | ||
| StorageManager.save(CT_EIT_FALLBACK, true); | ||
| this.logger.debug('EIT fallback flag set - subsequent requests will use JSONP'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Static method uses this.logger which may not work as expected.
setEITFallback() is a static method but uses this.logger. While RequestDispatcher.logger is defined as a static property (line 9725), using this in static context is confusing and can lead to issues if the method is called differently. Use RequestDispatcher.logger explicitly for clarity and correctness.
Proposed fix
static setEITFallback() {
if (StorageManager._isLocalStorageSupported()) {
StorageManager.save(CT_EIT_FALLBACK, true);
- this.logger.debug('EIT fallback flag set - subsequent requests will use JSONP');
+ RequestDispatcher.logger.debug('EIT fallback flag set - subsequent requests will use JSONP');
}
}🤖 Prompt for AI Agents
In `@clevertap.js` around lines 9306 - 9311, The static method setEITFallback uses
this.logger which is confusing in a static context; change it to reference the
static logger explicitly (RequestDispatcher.logger) so the log call always uses
the class-level logger. In the setEITFallback method (alongside StorageManager
and CT_EIT_FALLBACK references), replace this.logger.debug(...) with
RequestDispatcher.logger.debug(...) to avoid relying on dynamic this binding and
ensure clarity and correctness.
| function decryptFromBackend(envelopeB64) { | ||
| const { itp, itk, itv } = JSON.parse(LZS.decompressFromBase64(envelopeB64)); | ||
|
|
||
| const payload = Uint8Array.from(atob(itp), c => c.charCodeAt(0)); | ||
| const key = Uint8Array.from(atob(itk), c => c.charCodeAt(0)); | ||
| const iv = Uint8Array.from(atob(itv), c => c.charCodeAt(0)); | ||
|
|
||
| const alg = { name: 'AES-GCM', iv, tagLength: 128 }; | ||
|
|
||
| return crypto.subtle.importKey('raw', key, alg, false, ['decrypt']) | ||
| .then((keyObj) => crypto.subtle.decrypt(alg, keyObj, payload)) | ||
| .then((plainBuf) => new TextDecoder().decode(plainBuf)); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the actual decryptFromBackend implementation
echo "=== Checking decryptFromBackend implementation ==="
find . -type f -name "encryptionInTransit.js" | head -5Repository: CleverTap/clevertap-web-sdk
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Look for the implementation file
find . -type f -name "*.js" -path "*/security/*" | grep -i encryptRepository: CleverTap/clevertap-web-sdk
Length of output: 204
🏁 Script executed:
#!/bin/bash
# Check for test files
find . -type f -name "*encryptionInTransit*.spec.js" -o -name "*encryptionInTransit*.test.js"Repository: CleverTap/clevertap-web-sdk
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Check the decryptFromBackend implementation
echo "=== Implementation in encryptionInTransit.js ==="
cat -n ./src/util/security/encryptionInTransit.js | head -200Repository: CleverTap/clevertap-web-sdk
Length of output: 6243
🏁 Script executed:
#!/bin/bash
# Check the test file to see how the key is set
echo "=== Test file encryptionInTransit.spec.js ==="
cat -n ./test/unit/util/security/encryptionInTransit.spec.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 12230
🏁 Script executed:
#!/bin/bash
# Verify the Encryption-in-Transit.md file exists and check the documented code
find . -name "Encryption-in-Transit.md" -type fRepository: CleverTap/clevertap-web-sdk
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Check the documentation file around lines 114-127
sed -n '100,150p' ./Encryption-in-Transit.mdRepository: CleverTap/clevertap-web-sdk
Length of output: 3432
🏁 Script executed:
#!/bin/bash
# Double-check the envelope structure created by encryptForBackend to confirm itk is included
sed -n '87,93p' ./src/util/security/encryptionInTransit.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 381
Update documentation to reflect actual implementation: decryption uses the stored singleton key, not the key from the envelope.
The documented decryptFromBackend function (lines 114–127) shows extracting the encryption key from the envelope (itk), but the actual implementation (lines 109–155 of encryptionInTransit.js) uses this.encryptionKey from the singleton instance and does not extract or use itk from the envelope.
The envelope structure in tests omits itk entirely, and the backend response only needs to contain itp (ciphertext) and itv (IV) to be decrypted. Update the documented code snippet to reflect this: extract only itp and itv, and use the stored encryption key from the instance instead of extracting it from the envelope.
🤖 Prompt for AI Agents
In `@Encryption-in-Transit.md` around lines 114 - 127, The documentation snippet
for decryptFromBackend is out of sync: remove extraction of itk from the
envelope and update the code to parse only itp and itv, then use the stored
singleton key (this.encryptionKey / instance encryptionKey) instead of the
envelope key; specifically, change the JSON.parse destructuring to only { itp,
itv }, decode itp and itv as before, and call crypto.subtle.importKey/import
with the stored key (this.encryptionKey) and AES-GCM alg before decrypting the
payload in decryptFromBackend.
| export const ENCRYPTION_KEY_NAME = 'WZRK_ENCRYPTION_KEY' | ||
|
|
||
| // Flag key for optional sub-domain profile isolation | ||
| export const ISOLATE_COOKIE = 'WZRK_ISOLATE_SD' | ||
|
|
||
| // SDK Muting - Progressive muting for churned customers | ||
| export const MUTE_EXPIRY_KEY = 'WZRK_MUTE_EXPIRY' | ||
| // Flag key for Encryption in Transit JSONP fallback (session-level) | ||
| export const CT_EIT_FALLBACK = 'CT_EIT_FALLBACK' | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all definitions of the mentioned constants
echo "=== Searching for MUTE_EXPIRY_KEY definitions ==="
rg -n "MUTE_EXPIRY_KEY\s*=" --type=js -A1
echo -e "\n=== Searching for CT_EIT_FALLBACK definitions ==="
rg -n "CT_EIT_FALLBACK\s*=" --type=js -A1
echo -e "\n=== Searching for ISOLATE_COOKIE definitions ==="
rg -n "ISOLATE_COOKIE\s*=" --type=js -A1
echo -e "\n=== Checking if constants are imported in clevertap.js ==="
rg -n "import.*\{.*MUTE_EXPIRY_KEY|import.*\{.*CT_EIT_FALLBACK|import.*\{.*ISOLATE_COOKIE" src/clevertap.js
echo -e "\n=== Checking imports from constants.js in clevertap.js ==="
rg -n "from.*constants" src/clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 1145
🏁 Script executed:
#!/bin/bash
# Check what is actually imported from constants.js at line 42
sed -n '40,50p' src/clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 728
🏁 Script executed:
#!/bin/bash
# Get the full import block from constants.js
sed -n '30,50p' src/clevertap.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 924
🏁 Script executed:
#!/bin/bash
# Get earlier lines to see start of import block
sed -n '1,50p' src/clevertap.js | grep -A 30 "^import"Repository: CleverTap/clevertap-web-sdk
Length of output: 1793
Remove duplicate constant definitions in src/clevertap.js and import from src/util/constants.js instead.
All three constants (ISOLATE_COOKIE, MUTE_EXPIRY_KEY, CT_EIT_FALLBACK) are already exported from src/util/constants.js but are also defined as local const declarations in src/clevertap.js (lines 226, 228, 230).
ISOLATE_COOKIE is particularly problematic: it's already in the import statement from src/util/constants but is then immediately shadowed by a local definition at line 226—remove this redundant local const.
MUTE_EXPIRY_KEY and CT_EIT_FALLBACK should be added to the existing import from src/util/constants instead of being defined locally.
🤖 Prompt for AI Agents
In `@src/util/constants.js` around lines 75 - 84, Remove the redundant local
constant declarations for ISOLATE_COOKIE, MUTE_EXPIRY_KEY, and CT_EIT_FALLBACK
in src/clevertap.js (they currently shadow the imported ISOLATE_COOKIE and
duplicate the other two); instead, add MUTE_EXPIRY_KEY and CT_EIT_FALLBACK to
the existing import from src/util/constants so all three values come from the
shared exports, and verify all usages in clevertap.js reference the imported
symbols (no local consts remain).
| .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.
Suspicious Promise instanceof check after .then() resolution.
After a .then() callback, the value should already be resolved. Checking rawResponse instanceof Promise suggests the promise chain is broken somewhere. This check at line 326 (and similar at line 351) indicates control flow issues.
If line 313's recursive call is fixed, these checks may become unnecessary.
🤖 Prompt for AI Agents
In `@src/util/requestDispatcher.js` around lines 324 - 328, The
.then((rawResponse) => { ... }) callback in requestDispatcher.js contains a
redundant rawResponse instanceof Promise check that masks a broken promise
chain; fix the earlier recursive retry call (the retry path that re-invokes the
dispatcher) so it returns/propagates its Promise instead of breaking the chain,
then remove the instanceof Promise checks (and the corresponding duplicate at
the later .then) so the .then always receives a resolved value or null for JSONP
fallback. Ensure the retry function/method (the recursive invocation used for
retries) returns the Promise it creates so downstream code in the .then handlers
no longer needs to guard for Promise instances.
| const { tr, meta, wpe } = jsonResponse | ||
| if (tr) { | ||
| window.$WZRK_WR.tr(tr) | ||
| } | ||
| if (meta) { | ||
| const { g, sid, rf, rn, optOut } = meta | ||
| if (g && sid !== undefined && rf !== undefined && rn !== undefined) { | ||
| const parsedRn = parseInt(rn) | ||
|
|
||
| // Include optOut as 5th parameter if present | ||
| if (optOut !== undefined) { | ||
| window.$WZRK_WR.s(g, sid, rf, parsedRn, optOut) | ||
| } else { | ||
| window.$WZRK_WR.s(g, sid, rf, parsedRn) | ||
| } | ||
| } | ||
| } | ||
| if (wpe) { | ||
| window.$WZRK_WR.enableWebPush(wpe.enabled, wpe.key) | ||
| } |
There was a problem hiding this comment.
Add defensive check for window.$WZRK_WR.
Accessing window.$WZRK_WR.tr(), .s(), and .enableWebPush() without checking if $WZRK_WR exists could throw if the SDK is partially loaded or in an unexpected state.
🛡️ Proposed fix
const { tr, meta, wpe } = jsonResponse
- if (tr) {
+ if (tr && window.$WZRK_WR?.tr) {
window.$WZRK_WR.tr(tr)
}
- if (meta) {
+ if (meta && window.$WZRK_WR?.s) {
const { g, sid, rf, rn, optOut } = meta
// ...
}
- if (wpe) {
+ if (wpe && window.$WZRK_WR?.enableWebPush) {
window.$WZRK_WR.enableWebPush(wpe.enabled, wpe.key)
}🤖 Prompt for AI Agents
In `@src/util/requestDispatcher.js` around lines 364 - 383, The code calls
window.$WZRK_WR.tr, .s and .enableWebPush directly which can throw if the SDK
object is missing; update the block that reads jsonResponse (the references to
tr, meta, wpe and calls to window.$WZRK_WR.tr, window.$WZRK_WR.s,
window.$WZRK_WR.enableWebPush) to first guard that window.$WZRK_WR is defined
and that each method is a function before invoking it (e.g., if (window.$WZRK_WR
&& typeof window.$WZRK_WR.tr === 'function') window.$WZRK_WR.tr(tr); and similar
guards for .s and .enableWebPush), keeping the existing parameter handling
(including optOut) unchanged.
| toB64 (u8) { | ||
| return btoa(String.fromCharCode(...u8)) | ||
| } |
There was a problem hiding this comment.
Spread operator on large arrays can exceed call stack size.
Using String.fromCharCode(...u8) will fail for large payloads (typically >~100KB) due to "Maximum call stack size exceeded" error. Consider chunking the conversion.
🔧 Proposed fix using chunked conversion
toB64 (u8) {
- return btoa(String.fromCharCode(...u8))
+ // Process in chunks to avoid call stack overflow for large payloads
+ const CHUNK_SIZE = 0x8000 // 32KB chunks
+ let result = ''
+ for (let i = 0; i < u8.length; i += CHUNK_SIZE) {
+ result += String.fromCharCode.apply(null, u8.subarray(i, i + CHUNK_SIZE))
+ }
+ return btoa(result)
}📝 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.
| toB64 (u8) { | |
| return btoa(String.fromCharCode(...u8)) | |
| } | |
| toB64 (u8) { | |
| // Process in chunks to avoid call stack overflow for large payloads | |
| const CHUNK_SIZE = 0x8000 // 32KB chunks | |
| let result = '' | |
| for (let i = 0; i < u8.length; i += CHUNK_SIZE) { | |
| result += String.fromCharCode.apply(null, u8.subarray(i, i + CHUNK_SIZE)) | |
| } | |
| return btoa(result) | |
| } |
🤖 Prompt for AI Agents
In `@src/util/security/encryptionInTransit.js` around lines 19 - 21, The toB64
function uses String.fromCharCode(...u8), which breaks on large Uint8Array
inputs; update the toB64 implementation to convert the Uint8Array to a string in
chunks (e.g., loop over u8 in fixed-size slices and call String.fromCharCode on
each slice) and then call btoa on the concatenated string; ensure to handle
Uint8Array inputs and choose a chunk size that avoids exceeding call stack
(e.g., 32KB-ish) and preserve existing return behavior.
| const envelope = { | ||
| itp: this.toB64(cipher), // payload - base64 encoded ciphertext (includes auth tag) | ||
| itk: this.toB64(this.encryptionKey), // key - base64 encoded raw AES key | ||
| itv: this.toB64(iv), // iv - base64 encoded IV | ||
| id, | ||
| encrypted: true | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full file to understand the context
wc -l src/util/security/encryptionInTransit.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 115
🏁 Script executed:
# Get the full file content with line numbers
cat -n src/util/security/encryptionInTransit.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 6194
🏁 Script executed:
# Search for how this envelope is used/sent
rg -n "envelope" src/util/security/encryptionInTransit.js -A 5 -B 5Repository: CleverTap/clevertap-web-sdk
Length of output: 2125
🏁 Script executed:
# Search for where encryptForBackend is called
rg -rn "encryptForBackend" --type jsRepository: CleverTap/clevertap-web-sdk
Length of output: 1526
🏁 Script executed:
# Look for key exchange or asymmetric encryption mechanisms
rg -rn "public.*key\|private.*key\|RSA\|ECDH\|asymmetric" --type jsRepository: CleverTap/clevertap-web-sdk
Length of output: 53
🏁 Script executed:
# Search for TLS/HTTPS/transport security configuration
rg -rn "https\|tls\|TLS\|certificate\|trusted" --type js -i | head -30Repository: CleverTap/clevertap-web-sdk
Length of output: 53
🏁 Script executed:
# Check if there's documentation or security-related comments
rg -rn "security\|threat\|vulnerable\|attack" --type js src/util/security/ -B 2 -A 2Repository: CleverTap/clevertap-web-sdk
Length of output: 53
🏁 Script executed:
# Let's check if there's any documentation about threat model or key exchange
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "SECURITY*" -o -name "README*" \) | head -20Repository: CleverTap/clevertap-web-sdk
Length of output: 1088
🏁 Script executed:
# Check the full context of how this encryption is being used in requestDispatcher
sed -n '1,50p' src/util/requestDispatcher.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 1965
🏁 Script executed:
# Search for any comments about Phase 2 or TODO related to encryption
rg -n "Phase\|TODO\|FIXME" src/util/security/encryptionInTransit.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 53
🏁 Script executed:
# Check the encryption documentation
cat Encryption-in-Transit.mdRepository: CleverTap/clevertap-web-sdk
Length of output: 10784
🏁 Script executed:
# Look for backend-side implementation or documentation
find . -name "*backend*" -o -name "*server*" | grep -E "\.(js|ts|md)$" | head -10Repository: CleverTap/clevertap-web-sdk
Length of output: 725
Critical: Sending encryption key with ciphertext defeats encryption purpose.
The symmetric key (itk) is transmitted in the same envelope as the ciphertext (line 89). Anyone intercepting this request can trivially decrypt the payload using the included key, providing no application-layer confidentiality protection.
This is the documented Phase 1 design, but the architecture is flawed:
- No asymmetric key wrapping or separate key exchange mechanism
- Design does not explicitly justify reliance on TLS for security
- Decryption logic (line 135) uses locally stored key, not the transmitted one, suggesting incomplete/transitional implementation
Recommended fixes:
- Use asymmetric encryption (RSA/ECDH) to wrap the symmetric key before transmission
- Implement a separate secure key exchange handshake
- Explicitly document if security model assumes TLS + specific backend handling
Verify the intended security architecture and threat model with the backend team before Phase 2 implementation.
🤖 Prompt for AI Agents
In `@src/util/security/encryptionInTransit.js` around lines 87 - 93, The envelope
currently includes the raw symmetric key (itk = this.toB64(this.encryptionKey)),
which defeats confidentiality; instead stop serializing/transmitting
this.encryptionKey directly and implement asymmetric key-wrapping or
key-exchange: before building envelope, encrypt/wrap this.encryptionKey with the
recipient's public key (or derive a shared key via ECDH) and set itk to the
wrapped-key blob (base64) or omit itk entirely if using an established session
key; update the corresponding decryption path that consumes the envelope (the
code that reads itp/itk and the decryption routine referenced near the
decryption logic) to unwrap the symmetric key with the matching private key or
session secret and then decrypt itp with the unwrapped key; also add a short
comment documenting the chosen threat model (TLS-only vs. application-layer
key-wrapping) so backend/integration expectations are explicit.
| import { encryptForBackend, decryptFromBackend } from './src/util/security/encryptionInTransit.js' | ||
| import { decompressFromBase64 } from './src/util/encoder.js' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the actual export pattern in encryptionInTransit.js
echo "=== Checking exports in encryptionInTransit.js ==="
rg -n "^export" src/util/security/encryptionInTransit.js
echo ""
echo "=== Checking for default export ==="
rg -n "export default" src/util/security/encryptionInTransit.js
echo ""
echo "=== Checking end of file for exports ==="
tail -20 src/util/security/encryptionInTransit.jsRepository: CleverTap/clevertap-web-sdk
Length of output: 1019
Fix import statement to use default export.
The module encryptionInTransit.js exports only a default export (export default encryptionInTransitInstance at line 163), but the import statement uses named imports syntax. This will fail at runtime with an import error.
Change the import to:
import encryptionInTransitInstance from './src/util/security/encryptionInTransit.js'Then use encryptionInTransitInstance.encryptForBackend(...) and encryptionInTransitInstance.decryptFromBackend(...) in the code.
🤖 Prompt for AI Agents
In `@test-encryption.js` around lines 1 - 2, The current import uses named imports
encryptForBackend and decryptFromBackend but the module only exposes a default
export; change the import to default (import encryptionInTransitInstance from
'./src/util/security/encryptionInTransit.js') and update call sites to use
encryptionInTransitInstance.encryptForBackend(...) and
encryptionInTransitInstance.decryptFromBackend(...), ensuring all references to
encryptForBackend and decryptFromBackend are replaced accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/util/requestDispatcher.js`:
- Around line 280-287: In handleFetchResponse(encryptedUrl, originalUrl,
retryCount = 0) stop always setting the 'X-CleverTap-Encryption-Enabled' header
to 'true'; instead read the runtime flag (e.g. enableEncryptionInTransit or
equivalent config used elsewhere in requestDispatcher) and only add the header
when that flag is true and the payload is actually encrypted; update the headers
construction in handleFetchResponse to conditionally include
'X-CleverTap-Encryption-Enabled': 'true' based on that flag so unencrypted
responses aren’t advertised as encrypted.
| static handleFetchResponse (encryptedUrl, originalUrl, retryCount = 0) { | ||
| const fetchOptions = { | ||
| method: 'GET', | ||
| headers: { | ||
| Accept: 'application/json', | ||
| 'X-CleverTap-Encryption-Enabled': 'true' | ||
| } | ||
| } |
There was a problem hiding this comment.
Don’t advertise encryption when payload isn’t encrypted.
Line 285 always sets X-CleverTap-Encryption-Enabled: true, even when enableEncryptionInTransit is false (but Fetch is on). That can trigger server-side decryption errors and break response handling.
✅ Minimal fix
const fetchOptions = {
method: 'GET',
headers: {
Accept: 'application/json',
- 'X-CleverTap-Encryption-Enabled': 'true'
+ 'X-CleverTap-Encryption-Enabled': this.enableEncryptionInTransit ? 'true' : 'false'
}
}🤖 Prompt for AI Agents
In `@src/util/requestDispatcher.js` around lines 280 - 287, In
handleFetchResponse(encryptedUrl, originalUrl, retryCount = 0) stop always
setting the 'X-CleverTap-Encryption-Enabled' header to 'true'; instead read the
runtime flag (e.g. enableEncryptionInTransit or equivalent config used elsewhere
in requestDispatcher) and only add the header when that flag is true and the
payload is actually encrypted; update the headers construction in
handleFetchResponse to conditionally include 'X-CleverTap-Encryption-Enabled':
'true' based on that flag so unencrypted responses aren’t advertised as
encrypted.
2 participants
Changes
Added the logic to implement SDK muting
Changes to Public Facing API if any
No
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
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation