Skip to content

Conversation

@akashvercetti
Copy link
Collaborator

@akashvercetti akashvercetti commented Sep 15, 2025

  • Introduced a new encryption level enum to encrypt cached events, profiles and inbox messages.

Summary by CodeRabbit

  • New Features

    • Added a High encryption level for stronger protection of local data, queues, inbox messages and cached variables, with automatic migration between levels.
    • Automatically discovers registered URL schemes from app settings.
  • Refactor

    • Improved encryption/decryption resilience with per-item validation, safer fallbacks, and consistent propagation of encryption context across inbox and queue handling.

Note

Introduces a new High (2) encryption level and applies encrypt/decrypt with migration across queues, profiles, inbox messages, and Product Experiences variable diffs.

  • Encryption
    • Add CleverTapEncryptionHigh (2) to CleverTapEncryptionLevel and parse from Info.plist.
    • CTEncryptionManager: track previousEncryptionLevel, update cached GUIDs for Medium/High, expose AES-GCM helpers.
  • Queues & Profiles
    • Encrypt events, profile, and notifications queues on persist when level is High; decrypt on inflate if previously High with safe fallbacks.
    • Local profile: decrypt all fields if last level was High; encrypt all keys on High (vs PII-only on Medium).
  • Inbox
    • CTInboxController now initialized with encryption context; pre-encrypt incoming messages at High, migrate messages on level changes.
    • CTMessageMO: store encrypted json when applicable and decrypt in toJSON via CTUserMO.encryptionManager.
    • CTUserMO: carries encryptionManager; creation/updates wired to pass it through.
  • Product Experiences (Vars)
    • CTVarCache: encrypt diffs for storage at High, decrypt on load, and migrate diffs on level changes.

Written by Cursor Bugbot for commit 4553883. This will update automatically on new commits. Configure here.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 15, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a new High (2) encryption level and propagates encryption context across SDK components. Introduces full-dictionary AES-GCM encryption/decryption paths for profiles, queues, diffs, and inbox messages, migration logic for level changes, CTPlistInfo plist helpers/singleton, a previousEncryptionLevel field, and updated initializers/signatures to pass encryptionManager and levels.

Changes

Cohort / File(s) Summary
Encryption enum & build guard
CleverTapSDK/CleverTap.h
Adds CleverTapEncryptionHigh = 2 and changes HostWatchOS method declaration guard to #if defined(CLEVERTAP_HOST_WATCHOS).
Plist helpers / encryption resolution
CleverTapSDK/CTPlistInfo.m
Adds sharedInstance, getValueForKey:, getMetaDataForAttribute:, getRegisteredURLSchemes; reads Info.plist metadata with error handling; extends setEncryption: to recognize level 2 (High).
Encryption manager state
CleverTapSDK/Encryption/CTEncryptionManager.h, .../CTEncryptionManager.m
Adds previousEncryptionLevel property; updateEncryptionLevel stores prior level; identifier caching/encryption now treats High like Medium.
Local data store: profiles encryption
CleverTapSDK/CTLocalDataStore.m
Adds High-path to encrypt/decrypt all profile keys (not only PII) using cryptManager; retains Medium (PII-only) behavior; adds per-key type checks and try/catch logging.
Core instance: queues persistence/load & inbox wiring
CleverTapSDK/CleverTap.m
On load, decrypts archived queues when previousEncryptionLevel == High; on save, encrypts queues when encryptionLevel == High; passes encryptionLevel, previousEncryptionLevel, encryptionManager into CTInboxController initializers.
CTInboxController API & logic
CleverTapSDK/Inbox/controllers/CTInboxController.h, .../CTInboxController.m
Extends initializer to accept encryptionLevel, previousEncryptionLevel, and encryptionManager; stores encryption context; pre-processes and migrates per-message encryption on level change; adds helpers for migrate/detect/encrypt/decrypt message payloads.
Inbox CoreData: message payload handling
CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
initWithJSON: recognizes _ct_is_encrypted and stores _ct_encrypted_payload; toJSON transparently decrypts AES-GCM payloads using message's user's encryptionManager; adds +fetchRequest.
Inbox CoreData: user encryption plumbing
CleverTapSDK/Inbox/models/CTUserMO.h, .../CTUserMO.m, .../CTUserMO+CoreDataProperties.h, .../CTUserMO+CoreDataProperties.m
Adds encryptionManager property; fetchOrCreateFromJSON:... and initWithJSON:... updated to accept and assign encryptionManager, ensuring user objects hold the manager for message decryption.
Var cache encryption for feature flags/diffs
CleverTapSDK/ProductExperiences/CTVarCache.m
Adds private encryptionLevel and previousEncryptionLevel, decrypts diffs when loading, encrypts (wraps under _ct_encrypted_vars) when saving at High, and migrates diffs on level change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App
  participant CleverTap
  participant EncryptionManager as CTEncryptionManager
  participant Storage as LocalStore/Queues

  Note over CleverTap,EncryptionManager: Initialization
  App->>CleverTap: init(...)
  CleverTap->>EncryptionManager: updateEncryptionLevel()
  EncryptionManager-->>CleverTap: encryptionLevel, previousEncryptionLevel

  Note over CleverTap,Storage: Queue Inflate (Load)
  CleverTap->>Storage: read archived queues
  alt previousEncryptionLevel == High
    CleverTap->>EncryptionManager: decrypt(archivedData)
    EncryptionManager-->>CleverTap: plaintext or error
    opt error
      CleverTap-->>Storage: use empty queue
    end
  end

  Note over CleverTap,Storage: Queue Persist (Save)
  alt encryptionLevel == High
    CleverTap->>EncryptionManager: encrypt(queue copy)
    EncryptionManager-->>CleverTap: ciphertext
  end
  CleverTap->>Storage: archive and save
Loading
sequenceDiagram
  autonumber
  participant CleverTap
  participant Inbox as CTInboxController
  participant EncryptionManager as CTEncryptionManager
  participant CoreData

  Note over CleverTap,Inbox: Inbox Init
  CleverTap->>Inbox: init(accountId,guid,encryptionLevel,previousEncryptionLevel,encryptionManager)

  Note over Inbox,CoreData: Load & Migrate
  Inbox->>CoreData: fetch messages
  alt previous != current
    Inbox->>Inbox: migrateMessagesEncryption()
    loop each message
      alt None/Medium -> High
        Inbox->>EncryptionManager: encrypt(json)
        EncryptionManager-->>Inbox: encrypted payload
      else High -> None/Medium
        Inbox->>EncryptionManager: decrypt(encryptedPayload)
        EncryptionManager-->>Inbox: plaintext json
      end
      Inbox->>CoreData: save updated message
    end
  end

  Note over CoreData,Inbox: toJSON()
  CoreData-->>Inbox: (decrypted or plain) json for clients
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • darshanclevertap
  • nishant-clevertap
  • nzagorchev

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The pull request title "SDK- 5154: New Encryption Level High" directly and clearly summarizes the primary change in the changeset. The raw summary confirms that the main objective is to introduce a new encryption level enum (CleverTapEncryptionHigh = 2) across multiple SDK components including local data storage, inbox messages, cached events/profiles, and variables. The title is specific, concise, and includes the ticket identifier for tracking. While it doesn't enumerate every affected component, it accurately captures the core change in a way that a teammate scanning the PR history would immediately understand the purpose of this changeset.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch SDK-5154-encryption-high

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@akashvercetti
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 15, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@francispereira
Copy link

francispereira commented Sep 15, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (9)
CleverTapSDK/CTPlistInfo.m (2)

140-146: Nit: keep "} else ..." on the same line for consistency.

Matches surrounding style and reduces diff noise.

-    }
-    else if (encryptionLevel && [encryptionLevel isEqualToString:@"2"]) {
+    } else if (encryptionLevel && [encryptionLevel isEqualToString:@"2"]) {
         _encryptionLevel = CleverTapEncryptionHigh;
-    }
-    else {
+    } else {
         _encryptionLevel = CleverTapEncryptionNone;
         CleverTapLogStaticInternal(@"Supported encryption levels are only 0, 1 and 2. Setting it to 0 by default");

46-55: URL schemes collection overwrites earlier entries; accumulate instead.

Current loop replaces registeredURLSchemes on each iteration, keeping only the last item’s schemes.

-                        registeredURLSchemes = [cfBundleURLSchemes copy];
+                        registeredURLSchemes = [registeredURLSchemes arrayByAddingObjectsFromArray:cfBundleURLSchemes];

Optional: dedupe afterwards using NSOrderedSet if needed.

CleverTapSDK/Encryption/CTEncryptionManager.h (1)

32-32: Ensure proper initialization of previousEncryptionLevel property.

The new property lacks an explicit initial value declaration. While it will default to 0 (CleverTapEncryptionNone), consider explicitly initializing it in the implementation file to avoid potential undefined behavior.

CleverTapSDK/Inbox/models/CTUserMO.m (1)

4-4: Consider using auto-synthesis instead of explicit @synthesize.

Modern Objective-C automatically synthesizes properties with backing ivars. The explicit @synthesize encryptionManager; is unnecessary unless you need custom ivar naming.

Apply this diff to remove the unnecessary synthesis:

-@synthesize encryptionManager;
CleverTapSDK/Inbox/controllers/CTInboxController.h (1)

26-30: Consider nullable annotations for encryption parameters.

The encryptionManager parameter should have a nullable annotation to be consistent with the return type and other parameters. If it's required, consider using nonnull instead.

Apply this diff to add proper nullability annotation:

- (instancetype _Nullable)initWithAccountId:(NSString *)accountId
                                       guid:(NSString *)guid
                            encryptionLevel:(CleverTapEncryptionLevel)encryptionLevel
                    previousEncryptionLevel:(CleverTapEncryptionLevel)previousEncryptionLevel
-                          encryptionManager:(CTEncryptionManager*)encryptionManager;
+                          encryptionManager:(CTEncryptionManager * _Nonnull)encryptionManager;
CleverTapSDK/CTLocalDataStore.m (2)

1025-1059: Code duplication in encryption/decryption logic

The High and Medium encryption level paths have nearly identical code structure. Consider refactoring to reduce duplication and improve maintainability.

+- (NSMutableDictionary *)decryptProfileData:(NSMutableDictionary *)profile withKeys:(NSArray *)keysToDecrypt {
+    NSMutableDictionary *updatedProfile = [NSMutableDictionary new];
+    
+    for (NSString *key in profile) {
+        BOOL shouldDecrypt = keysToDecrypt ? [keysToDecrypt containsObject:key] : YES;
+        
+        if (shouldDecrypt) {
+            @try {
+                // Validate the value before attempting to decrypt
+                id value = profile[key];
+                if (!value || ![value isKindOfClass:[NSString class]]) {
+                    CleverTapLogDebug(self.config.logLevel, @"%@: Invalid value for key: %@, skipping decryption", self, key);
+                    updatedProfile[key] = value ?: [NSNull null];
+                    continue;
+                }
+                
+                NSString *stringValue = [NSString stringWithFormat:@"%@", value];
+                NSString *decryptedString = [self.config.cryptManager decryptString:stringValue];
+                
+                // Validate decryption result
+                if (!decryptedString) {
+                    CleverTapLogDebug(self.config.logLevel, @"%@: Failed to decrypt data for key: %@", self, key);
+                    // Return original value if decryption fails
+                    updatedProfile[key] = stringValue;
+                } else {
+                    updatedProfile[key] = decryptedString;
+                }
+            } @catch (NSException *e) {
+                CleverTapLogDebug(self.config.logLevel, @"%@: Exception during decryption for key %@: %@", self, key, e);
+                // Add original value to avoid data loss
+                updatedProfile[key] = profile[key];
+            }
+        } else {
+            updatedProfile[key] = profile[key];
+        }
+    }
+    
+    return updatedProfile;
+}

 - (NSMutableDictionary *)decryptPIIDataIfEncrypted:(NSMutableDictionary *)profile {
     // ... existing validation code ...
     
     if (lastEncryptionLevel == CleverTapEncryptionMedium && self.config.cryptManager) {
-        // existing Medium encryption code
+        return [self decryptProfileData:profile withKeys:_piiKeys];
     }
     else if (lastEncryptionLevel == CleverTapEncryptionHigh && self.config.cryptManager) {
-        // existing High encryption code
+        return [self decryptProfileData:profile withKeys:nil];
     }
     
     return profile;
 }

1077-1084: Consider extracting common encryption logic

Similar to decryption, the encryption logic for Medium and High levels shares common patterns that could be refactored.

+- (NSMutableDictionary *)encryptProfileData:(NSMutableDictionary *)profile withKeys:(NSArray *)keysToEncrypt {
+    NSMutableDictionary *updatedProfile = [NSMutableDictionary new];
+    for (NSString *key in profile) {
+        BOOL shouldEncrypt = keysToEncrypt ? [keysToEncrypt containsObject:key] : YES;
+        if (shouldEncrypt) {
+            NSString *value = [NSString stringWithFormat:@"%@", profile[key]];
+            updatedProfile[key] = [self.config.cryptManager encryptString:value];
+        } else {
+            updatedProfile[key] = profile[key];
+        }
+    }
+    return updatedProfile;
+}
CleverTapSDK/Inbox/controllers/CTInboxController.m (1)

320-363: Consider adding error logging for migration failures

The migration logic handles encryption level transitions well, but could benefit from more detailed error logging when encryption/decryption fails during migration.

         if (msg.json && ![self isJSONPropertyEncrypted:msg.json]) {
             NSString *encryptedJSON = [self.encryptionManager encryptObject:msg.json];
             if (encryptedJSON) {
                 msg.json = encryptedJSON;
                 CleverTapLogStaticDebug(@"Encrypted inbox message json for message ID: %@", msg.id);
+            } else {
+                CleverTapLogStaticDebug(@"Failed to encrypt inbox message json for message ID: %@ during migration", msg.id);
             }
         }
CleverTapSDK/CleverTap.m (1)

2127-2161: Consider extracting encryption logic to reduce duplication

The three persist queue methods have identical encryption logic. Consider extracting a common method.

+- (void)persistQueue:(NSMutableArray *)queue toFile:(NSString *)fileName {
+    id queueCopy;
+    @synchronized (self) {
+        queueCopy = [NSMutableArray arrayWithArray:[queue copy]];
+        if (self.config.encryptionLevel == CleverTapEncryptionHigh) {
+            queueCopy = [self.config.cryptManager encryptObject:queueCopy];
+        }
+    }
+    [CTPreferences archiveObject:queueCopy forFileName:fileName config:_config];
+}

 - (void)persistEventsQueue {
-    NSString *fileName = [self eventsFileName];
-    id eventsCopy;
-    @synchronized (self) {
-        eventsCopy = [NSMutableArray arrayWithArray:[self.eventsQueue copy]];
-        if (self.config.encryptionLevel == CleverTapEncryptionHigh) {
-            eventsCopy = [self.config.cryptManager encryptObject:eventsCopy];
-        }
-    }
-    [CTPreferences archiveObject:eventsCopy forFileName:fileName config:_config];
+    [self persistQueue:self.eventsQueue toFile:[self eventsFileName]];
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03370fb and 311f67a.

📒 Files selected for processing (13)
  • CleverTapSDK/CTLocalDataStore.m (3 hunks)
  • CleverTapSDK/CTPlistInfo.m (1 hunks)
  • CleverTapSDK/CleverTap.h (3 hunks)
  • CleverTapSDK/CleverTap.m (5 hunks)
  • CleverTapSDK/Encryption/CTEncryptionManager.h (1 hunks)
  • CleverTapSDK/Encryption/CTEncryptionManager.m (2 hunks)
  • CleverTapSDK/Inbox/controllers/CTInboxController.h (2 hunks)
  • CleverTapSDK/Inbox/controllers/CTInboxController.m (5 hunks)
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m (3 hunks)
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h (1 hunks)
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m (4 hunks)
  • CleverTapSDK/Inbox/models/CTUserMO.h (1 hunks)
  • CleverTapSDK/Inbox/models/CTUserMO.m (1 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/CTLocalDataStore.m:0-0
Timestamp: 2025-04-17T11:28:19.496Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), error handling has been implemented for encryption/decryption operations to handle potential failures when working with PII data.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTCryptHandler.m:0-0
Timestamp: 2025-04-17T10:51:39.897Z
Learning: The file `CleverTapSDK/Encryption/CTCryptHandler.m` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTEncryptionManager.m:310-347
Timestamp: 2025-04-17T11:19:10.966Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), the implementation in CTEncryptionManager.m maintains the old AES-CBC implementation with static IV alongside the new AES-GCM implementation to support migration of existing encrypted data.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401, replaced by `CTAESGCMCrypt.swift` which implements more secure key management using the Keychain.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTEncryptionManager.m:12-16
Timestamp: 2025-04-18T04:39:04.876Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), the hardcoded cryptographic constants in CTEncryptionManager.m (kCRYPT_KEY_PREFIX, kCRYPT_KEY_SUFFIX) are part of the legacy AES-CBC encryption that is being maintained only for migration purposes as the SDK transitions to AES-GCM encryption.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:34:46.988Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` previously used CBC mode without integrity verification, but has been completely replaced with `CTAESGCMCrypt.swift` using AES-GCM (authenticated encryption) in PR #401 to properly address data integrity concerns.
Learnt from: akashvercetti
PR: CleverTap/clevertap-ios-sdk#438
File: CleverTapSDK/Encryption/NetworkEncryptionManager.swift:15-28
Timestamp: 2025-06-03T16:29:34.418Z
Learning: In CleverTap iOS SDK, encryption in transit (PR #438) intentionally generates new session keys on each app launch for forward secrecy, which is different from the persistent key approach used for data at rest encryption (PR #401). The NetworkEncryptionManager is designed for temporary encryption of network communications to the backend, not for persistent data storage.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.m:0-0
Timestamp: 2025-04-17T10:49:43.627Z
Learning: In the CleverTap iOS SDK, the previous AES-CBC implementation in CTAESCrypt.m was intentionally kept untouched (despite security issues like static IVs) during the encryption upgrade to AES-GCM to facilitate the migration of existing encrypted data.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-15T05:02:57.045Z
Learning: CleverTap iOS SDK uses both AES-CBC (old) and AES-GCM (new) implementations during the encryption algorithm upgrade, with the older implementation maintained specifically for migration purposes.
📚 Learning: 2025-04-17T11:19:10.966Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTEncryptionManager.m:310-347
Timestamp: 2025-04-17T11:19:10.966Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), the implementation in CTEncryptionManager.m maintains the old AES-CBC implementation with static IV alongside the new AES-GCM implementation to support migration of existing encrypted data.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/Inbox/models/CTUserMO.m
  • CleverTapSDK/Inbox/models/CTUserMO.h
  • CleverTapSDK/Inbox/controllers/CTInboxController.h
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-04-18T04:39:04.876Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTEncryptionManager.m:12-16
Timestamp: 2025-04-18T04:39:04.876Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), the hardcoded cryptographic constants in CTEncryptionManager.m (kCRYPT_KEY_PREFIX, kCRYPT_KEY_SUFFIX) are part of the legacy AES-CBC encryption that is being maintained only for migration purposes as the SDK transitions to AES-GCM encryption.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/Inbox/models/CTUserMO.m
  • CleverTapSDK/Inbox/models/CTUserMO.h
  • CleverTapSDK/Inbox/controllers/CTInboxController.h
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-04-17T10:51:39.897Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTCryptHandler.m:0-0
Timestamp: 2025-04-17T10:51:39.897Z
Learning: The file `CleverTapSDK/Encryption/CTCryptHandler.m` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/Inbox/models/CTUserMO.m
  • CleverTapSDK/Inbox/models/CTUserMO.h
  • CleverTapSDK/Inbox/controllers/CTInboxController.h
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-04-17T10:49:43.627Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.m:0-0
Timestamp: 2025-04-17T10:49:43.627Z
Learning: In the CleverTap iOS SDK, the previous AES-CBC implementation in CTAESCrypt.m was intentionally kept untouched (despite security issues like static IVs) during the encryption upgrade to AES-GCM to facilitate the migration of existing encrypted data.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/Inbox/models/CTUserMO.m
  • CleverTapSDK/Inbox/models/CTUserMO.h
  • CleverTapSDK/Inbox/controllers/CTInboxController.h
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-04-17T10:28:14.900Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401, replaced by `CTAESGCMCrypt.swift` which implements more secure key management using the Keychain.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/Inbox/models/CTUserMO.m
  • CleverTapSDK/Inbox/models/CTUserMO.h
  • CleverTapSDK/Inbox/controllers/CTInboxController.h
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-04-17T10:34:46.988Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:34:46.988Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` previously used CBC mode without integrity verification, but has been completely replaced with `CTAESGCMCrypt.swift` using AES-GCM (authenticated encryption) in PR #401 to properly address data integrity concerns.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/Inbox/models/CTUserMO.m
  • CleverTapSDK/Inbox/models/CTUserMO.h
  • CleverTapSDK/Inbox/controllers/CTInboxController.h
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-04-17T10:28:14.900Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/Inbox/models/CTUserMO.m
  • CleverTapSDK/Inbox/models/CTUserMO.h
  • CleverTapSDK/Inbox/controllers/CTInboxController.h
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-04-17T11:28:19.496Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/CTLocalDataStore.m:0-0
Timestamp: 2025-04-17T11:28:19.496Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), error handling has been implemented for encryption/decryption operations to handle potential failures when working with PII data.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/Inbox/models/CTUserMO.m
  • CleverTapSDK/Inbox/models/CTUserMO.h
  • CleverTapSDK/Inbox/controllers/CTInboxController.h
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-06-03T16:29:34.418Z
Learnt from: akashvercetti
PR: CleverTap/clevertap-ios-sdk#438
File: CleverTapSDK/Encryption/NetworkEncryptionManager.swift:15-28
Timestamp: 2025-06-03T16:29:34.418Z
Learning: In CleverTap iOS SDK, encryption in transit (PR #438) intentionally generates new session keys on each app launch for forward secrecy, which is different from the persistent key approach used for data at rest encryption (PR #401). The NetworkEncryptionManager is designed for temporary encryption of network communications to the backend, not for persistent data storage.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-04-15T05:02:57.045Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-15T05:02:57.045Z
Learning: CleverTap iOS SDK uses both AES-CBC (old) and AES-GCM (new) implementations during the encryption algorithm upgrade, with the older implementation maintained specifically for migration purposes.

Applied to files:

  • CleverTapSDK/Encryption/CTEncryptionManager.h
  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/Encryption/CTEncryptionManager.m
  • CleverTapSDK/CleverTap.m
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-04-17T10:30:45.773Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTKeychainManager.swift:0-0
Timestamp: 2025-04-17T10:30:45.773Z
Learning: The file CTKeychainManager.swift was removed in PR #401 and its keychain operations functionality was consolidated into CTAESGCMCrypt.swift as part of the encryption algorithm upgrade to AES-GCM, which provides improved key management using the Keychain.

Applied to files:

  • CleverTapSDK/Inbox/models/CTUserMO.m
  • CleverTapSDK/Inbox/models/CTUserMO.h
  • CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m
📚 Learning: 2025-06-26T07:29:08.854Z
Learnt from: Sonal-Kachare
PR: CleverTap/clevertap-ios-sdk#447
File: SwiftUIStarter/SwiftUIStarter/CTAppInboxCustomView.swift:34-34
Timestamp: 2025-06-26T07:29:08.854Z
Learning: In SwiftUI CleverTap integration, when InboxMessage objects are created from CleverTapInboxMessage (the SDK's own message type), the message IDs are guaranteed to be valid and non-empty, so additional guard checks before calling CleverTap SDK methods are not required.

Applied to files:

  • CleverTapSDK/Inbox/controllers/CTInboxController.h
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
📚 Learning: 2025-06-26T07:30:49.414Z
Learnt from: Sonal-Kachare
PR: CleverTap/clevertap-ios-sdk#447
File: SwiftStarter/SwiftStarter/Supporting Files/AppInboxTableViewCell.swift:34-34
Timestamp: 2025-06-26T07:30:49.414Z
Learning: In CleverTap iOS SDK integrations (both SwiftUI and UIKit), when objects are created from CleverTapInboxMessage (the SDK's own message type), the message IDs are guaranteed to be valid and non-empty, so additional guard checks before calling CleverTap SDK methods like recordInboxNotificationClickedEvent, recordInboxNotificationViewedEvent, markReadInboxMessage, and deleteInboxMessage are not required.

Applied to files:

  • CleverTapSDK/Inbox/controllers/CTInboxController.h
📚 Learning: 2025-06-03T16:25:54.721Z
Learnt from: akashvercetti
PR: CleverTap/clevertap-ios-sdk#438
File: CleverTapSDK/Encryption/NetworkEncryptionManager.swift:0-0
Timestamp: 2025-06-03T16:25:54.721Z
Learning: In the CleverTap iOS SDK NetworkEncryptionManager.swift, the encrypt() and decrypt() methods return empty values on failure rather than using Result types or throwing errors due to Objective-C interoperability constraints, as Swift Result types don't bridge cleanly to Objective-C.

Applied to files:

  • CleverTapSDK/CleverTap.h
  • CleverTapSDK/CTPlistInfo.m
📚 Learning: 2025-01-23T07:03:18.609Z
Learnt from: darshanclevertap
PR: CleverTap/clevertap-ios-sdk#399
File: CleverTapSDK/EventDatabase/CTEventDatabase.m:480-490
Timestamp: 2025-01-23T07:03:18.609Z
Learning: The CTEventDatabase in CleverTap iOS SDK uses dispatch semaphores and synchronous execution during initialization to ensure thread safety in database operations.

Applied to files:

  • CleverTapSDK/CTLocalDataStore.m
📚 Learning: 2025-01-23T07:03:18.609Z
Learnt from: darshanclevertap
PR: CleverTap/clevertap-ios-sdk#399
File: CleverTapSDK/EventDatabase/CTEventDatabase.m:480-490
Timestamp: 2025-01-23T07:03:18.609Z
Learning: The CTEventDatabase in CleverTap iOS SDK ensures thread safety through semaphores, direct execution on serial queue, timeout handling, and thread-safe singleton pattern using dispatch_once.

Applied to files:

  • CleverTapSDK/CTLocalDataStore.m
  • CleverTapSDK/CleverTap.m
🔇 Additional comments (11)
CleverTapSDK/CTPlistInfo.m (1)

135-147: Encryption level "2" mapping looks correct — verify enum propagation and switches

  • The provided search returned no matches for an NS_ENUM definition of CleverTapEncryption or for CleverTapEncryptionHigh; confirm the enum is declared in a public header and exported to consumers.
  • Verify every codepath that branches on CleverTapEncryption (data store, inbox, queues, CTEncryptionManager, migration code) handles the new CleverTapEncryptionHigh value and that any required migration logic is implemented.
CleverTapSDK/Encryption/CTEncryptionManager.m (2)

87-88: LGTM! Previous encryption level tracking implemented correctly.

The implementation correctly captures the previous encryption level before updating to the new one, which will enable proper migration paths for encrypted data.


346-347: Good enhancement for High encryption level support.

The condition now correctly handles both Medium and High encryption levels for identifier encryption. This aligns well with the new three-tier encryption model.

CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.h (1)

9-9: LGTM! Method signature properly updated for encryption support.

The updated method signature correctly includes the encryptionManager parameter, enabling proper encryption context propagation for inbox message handling.

CleverTapSDK/CleverTap.h (2)

67-68: LGTM! New High encryption level added correctly.

The addition of CleverTapEncryptionHigh = 2 follows the proper enum value progression and aligns with the encryption enhancements across the SDK.

Based on the learnings, this continues the SDK's evolution from legacy AES-CBC to AES-GCM encryption, now with a more granular level of control.


1393-1397: Verify CLEVERTAP_HOST_WATCHOS is defined in build settings (WatchOS support)

CleverTapSDK/CleverTap.h now uses #if defined(CLEVERTAP_HOST_WATCHOS) (lines 1393–1397) but a repo search shows no definition of CLEVERTAP_HOST_WATCHOS and no xcconfig/pbxproj entries; CleverTapWatchOS/ files exist. Ensure CLEVERTAP_HOST_WATCHOS is set in the SDK’s podspec/xcconfig/Xcode project (or revert to TARGET_OS_WATCH) so the WatchOS method isn’t accidentally excluded.

CleverTapSDK/Inbox/models/CTUserMO+CoreDataProperties.m (1)

53-71: LGTM! Clean implementation of encryption manager integration

The initialization properly associates the encryption manager with the CTUserMO instance, ensuring encryption context is available throughout the object's lifecycle.

CleverTapSDK/Inbox/controllers/CTInboxController.m (2)

107-137: LGTM! Well-structured pre-encryption logic

The processMessagesForEncryption method properly handles encryption of inbox messages while preserving lookup fields. Good error handling with fallback to unencrypted storage on failure.


299-305: Persist previousEncryptionLevel across app restarts

previousEncryptionLevel is updated only in memory after migrateMessagesEncryption; persist it (e.g., UserDefaults or secure store) immediately after migration so an app restart during migration does not trigger repeated or partial migrations.

Location: CleverTapSDK/Inbox/controllers/CTInboxController.m (around lines 299–305).

CleverTapSDK/CleverTap.m (2)

2036-2094: LGTM! Robust queue inflation with encryption support

The queue inflation methods properly handle decryption of previously encrypted queues with appropriate fallbacks. Good defensive coding with the fallback to empty arrays when decryption fails.


3704-3704: Verify encryption level propagation in initialization chain

CTInboxController's initializer now requires encryptionLevel, previousEncryptionLevel and encryptionManager — ensure every CTInboxController allocation/initialization forwards these. Check CleverTapSDK/CleverTap.m (around lines ~3704 and ~3859) and any factory/wrapper methods (e.g. newInboxViewControllerWithConfig/newInboxViewController). I couldn't find other init sites in the repo; manual verification required.


@interface CTUserMO : NSManagedObject

@property (nonatomic, strong) CTEncryptionManager *encryptionManager;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Property should be declared as Core Data transient.

Since CTUserMO is a Core Data managed object and encryptionManager is not a persistent attribute, this property should be marked as transient to avoid Core Data warnings or potential persistence issues.

Based on the context from the previous learnings about encryption handling in the SDK, the encryptionManager should not be persisted but rather passed through during runtime operations.

🤖 Prompt for AI Agents
In CleverTapSDK/Inbox/models/CTUserMO.h around line 6, the encryptionManager
property is currently treated as a persistent Core Data attribute; mark it as
transient so it is not persisted. Fix by removing or marking this attribute as
transient in the .xcdatamodel (or remove it from the model entirely) and keep
encryptionManager as a runtime-only property (move its declaration to a
non-managed class extension in the .m or ensure it is not declared as an
NSManaged property), so Core Data will not attempt to persist it.

@akashvercetti
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 15, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (3)
CleverTapSDK/ProductExperiences/CTVarCache.m (1)

31-33: Init is fine, but keep these fields in sync thereafter.

See comment on Lines 18–19; ensure subsequent saves read from config and also update config.cryptManager.previousEncryptionLevel after migration.

CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m (2)

20-26: Prior feedback addressed: missing payload check added

The presence check for _ct_encrypted_payload looks good and prevents overwriting with nil.


68-71: Prior feedback addressed: graceful decryption failure

Returning a minimal dictionary on decrypt failure avoids downstream issues. Good.

🧹 Nitpick comments (10)
CleverTapSDK/ProductExperiences/CTVarCache.m (5)

200-204: Type-check unarchived payload before decrypting.

decodeObjectForKey: can return non-dictionary or nil. Guard the cast to avoid crashes and handle corrupted archives gracefully.

Apply:

-        NSDictionary *loadedDiffs = (NSDictionary *) [unarchiver decodeObjectForKey:CLEVERTAP_DEFAULTS_VARIABLES_KEY];
+        id loaded = [unarchiver decodeObjectForKey:CLEVERTAP_DEFAULTS_VARIABLES_KEY];
+        NSDictionary *loadedDiffs = [loaded isKindOfClass:[NSDictionary class]] ? loaded : @{};
         
         // Decrypt diffs if they were encrypted
         NSDictionary *diffs = [self decryptDiffsIfNeeded:loadedDiffs];

236-237: Combine file protection with atomic writes.

When file protection is enabled, you currently lose atomicity. The options are bitmaskable; keep atomic writes for durability.

Apply:

-        NSDataWritingOptions fileProtectionOption = _config.enableFileProtection ?
-        NSDataWritingFileProtectionComplete : NSDataWritingAtomic;
+        NSDataWritingOptions fileProtectionOption = _config.enableFileProtection ?
+            (NSDataWritingAtomic | NSDataWritingFileProtectionComplete) :
+            NSDataWritingAtomic;

394-411: Use the class’s level field consistently; avoid config/property mismatch.

Gate on self.encryptionLevel (kept in sync in save) to avoid divergence with config.encryptionLevel.

Apply:

-- (NSDictionary *)encryptDiffsIfNeeded:(NSDictionary *)diffs {
-    if (self.config.encryptionLevel != CleverTapEncryptionHigh || !self.config.cryptManager) {
+-- (NSDictionary *)encryptDiffsIfNeeded:(NSDictionary *)diffs {
+    if (self.encryptionLevel != CleverTapEncryptionHigh || !self.config.cryptManager) {
         return diffs;
     }

Optional (nice-to-have): define a shared key constant to avoid magic strings across classes:

// Near top of file
static NSString *const kCTEncryptedVarsKey = @"_ct_encrypted_vars";

433-468: Persist migration and simplify logic; avoid unused mutable copy.

  • Persist previousEncryptionLevel to config.cryptManager after migration (handled in save).
  • migratedDiffs isn’t needed; operate on self.diffs directly.
  • When downgrading from High, decrypt unconditionally if sentinel is present (leveraging the fixed decryption).

Apply:

 - (void)migrateVariablesEncryption {
     if (!self.diffs || [self.diffs count] == 0) {
         return;
     }
 
     CleverTapLogStaticInternal(@"Migrating variables encryption from level %d to %d",
                               (int)self.previousEncryptionLevel, (int)self.encryptionLevel);
-    // Create mutable copy for migration
-    NSMutableDictionary *migratedDiffs = [self.diffs mutableCopy];
 
     // Scenario 1: None/Medium → High (encrypt)
     if ((self.previousEncryptionLevel == CleverTapEncryptionNone ||
          self.previousEncryptionLevel == CleverTapEncryptionMedium) &&
         self.encryptionLevel == CleverTapEncryptionHigh) {
         
         // If diffs are not encrypted, they will be encrypted in saveDiffs
         CleverTapLogStaticInternal(@"Variables will be encrypted on next save");
     }
     
     // Scenario 2: High → None/Medium (decrypt)
     else if (self.previousEncryptionLevel == CleverTapEncryptionHigh &&
              (self.encryptionLevel == CleverTapEncryptionNone ||
               self.encryptionLevel == CleverTapEncryptionMedium)) {
         
         // If diffs are encrypted, decrypt them
-        if (migratedDiffs[@"_ct_encrypted_vars"]) {
-            NSDictionary *decryptedDiffs = [self decryptDiffsIfNeeded:migratedDiffs];
+        if (self.diffs[@"_ct_encrypted_vars"]) {
+            NSDictionary *decryptedDiffs = [self decryptDiffsIfNeeded:self.diffs];
             if (decryptedDiffs && decryptedDiffs != migratedDiffs) {
                 self.diffs = decryptedDiffs;
                 CleverTapLogStaticInternal(@"Decrypted variables for level change");
             }
         }
     }
 }

394-431: Add basic tests for round‑trip and migration.

  • Save at High → load → decrypt → merge OK.
  • Saved CBC payload (pre‑GCM) → load/decrypt OK.
  • High→None migration decrypts and persists previous level to cryptManager.

I can add unit tests in the ProductExperiences cache test target to cover these scenarios. Want me to open a follow‑up PR?

CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m (1)

20-26: Validate _ct_encrypted_payload type and emptiness

Guard for expected NSString payload; avoid assigning non-strings or empty strings to self.json.

Apply:

-            if (!json[@"_ct_encrypted_payload"]) {
+            id payload = json[@"_ct_encrypted_payload"];
+            if (!payload || ![payload isKindOfClass:[NSString class]] || [(NSString *)payload length] == 0) {
                 CleverTapLogStaticDebug(@"Message marked as encrypted but missing _ct_encrypted_payload");
                 self.json = [json copy];
             }
             else {
-                self.json = json[@"_ct_encrypted_payload"];
+                self.json = (NSString *)payload;
             }
CleverTapSDK/Inbox/controllers/CTInboxController.m (4)

33-48: Initializer: ensure callers migrated; provide assertion for nonnull manager when High

Since encryptionManager is required for High, assert to fail fast in debug.

Apply:

 if (self) {
@@
-            _encryptionLevel = encryptionLevel;
+            _encryptionLevel = encryptionLevel;
             _previousEncryptionLevel = previousEncryptionLevel;
             _encryptionManager = encryptionManager;
+            NSCAssert((_encryptionLevel != CleverTapEncryptionHigh) || (_encryptionManager != nil),
+                      @"CTEncryptionManager must be provided for CleverTapEncryptionHigh");

107-137: Avoid double-encryption of already-prepared messages

Skip items already marked _ct_is_encrypted.

Apply:

     for (NSDictionary *message in messages) {
+        if ([message[@"_ct_is_encrypted"] boolValue]) {
+            [processedMessages addObject:message];
+            continue;
+        }
         // Encrypt the message dictionary and create a wrapper
         NSString *encryptedJSON = [self.encryptionManager encryptObject:message];

298-306: Migration should no-op without a manager

Guard to avoid partial/inconsistent states when manager is absent.

Apply:

-    if (self.encryptionLevel != self.previousEncryptionLevel && [self.user.messages count] > 0) {
-        [self migrateMessagesEncryption];
+    if (self.encryptionLevel != self.previousEncryptionLevel && [self.user.messages count] > 0) {
+        if (!self.encryptionManager) {
+            CleverTapLogStaticDebug(@"Skipping inbox messages migration: missing CTEncryptionManager");
+        } else {
+            [self migrateMessagesEncryption];
+        }

331-360: Type-guard msg.json during migration

Encrypt only dictionaries; decrypt only strings.

Apply:

-    if ((fromLevel == CleverTapEncryptionNone || fromLevel == CleverTapEncryptionMedium) &&
+    if ((fromLevel == CleverTapEncryptionNone || fromLevel == CleverTapEncryptionMedium) &&
         toLevel == CleverTapEncryptionHigh) {
-        if (msg.json && ![self isJSONPropertyEncrypted:msg.json]) {
-            NSString *encryptedJSON = [self.encryptionManager encryptObject:msg.json];
+        if (msg.json && ![self isJSONPropertyEncrypted:msg.json] && [msg.json isKindOfClass:[NSDictionary class]]) {
+            NSString *encryptedJSON = [self.encryptionManager encryptObject:(NSDictionary *)msg.json];
             if (encryptedJSON) {
                 msg.json = encryptedJSON;
                 CleverTapLogStaticDebug(@"Encrypted inbox message json for message ID: %@", msg.id);
             }
         }
     }
@@
-    else if (fromLevel == CleverTapEncryptionHigh &&
+    else if (fromLevel == CleverTapEncryptionHigh &&
              (toLevel == CleverTapEncryptionNone || toLevel == CleverTapEncryptionMedium)) {
-        if (msg.json && [self isJSONPropertyEncrypted:msg.json]) {
-            id decryptedJSON = [self.encryptionManager decryptObject:(NSString *)msg.json];
+        if (msg.json && [msg.json isKindOfClass:[NSString class]] && [self isJSONPropertyEncrypted:msg.json]) {
+            id decryptedJSON = [self.encryptionManager decryptObject:(NSString *)msg.json];
             if (decryptedJSON) {
                 msg.json = decryptedJSON;
                 CleverTapLogStaticDebug(@"Decrypted inbox message json for message ID: %@", msg.id);
             }
         }
     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 311f67a and 8ae7886.

📒 Files selected for processing (3)
  • CleverTapSDK/Inbox/controllers/CTInboxController.m (5 hunks)
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m (3 hunks)
  • CleverTapSDK/ProductExperiences/CTVarCache.m (6 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/CTLocalDataStore.m:0-0
Timestamp: 2025-04-17T11:28:19.496Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), error handling has been implemented for encryption/decryption operations to handle potential failures when working with PII data.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTCryptHandler.m:0-0
Timestamp: 2025-04-17T10:51:39.897Z
Learning: The file `CleverTapSDK/Encryption/CTCryptHandler.m` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTEncryptionManager.m:310-347
Timestamp: 2025-04-17T11:19:10.966Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), the implementation in CTEncryptionManager.m maintains the old AES-CBC implementation with static IV alongside the new AES-GCM implementation to support migration of existing encrypted data.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401, replaced by `CTAESGCMCrypt.swift` which implements more secure key management using the Keychain.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTEncryptionManager.m:12-16
Timestamp: 2025-04-18T04:39:04.876Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), the hardcoded cryptographic constants in CTEncryptionManager.m (kCRYPT_KEY_PREFIX, kCRYPT_KEY_SUFFIX) are part of the legacy AES-CBC encryption that is being maintained only for migration purposes as the SDK transitions to AES-GCM encryption.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:34:46.988Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` previously used CBC mode without integrity verification, but has been completely replaced with `CTAESGCMCrypt.swift` using AES-GCM (authenticated encryption) in PR #401 to properly address data integrity concerns.
Learnt from: akashvercetti
PR: CleverTap/clevertap-ios-sdk#438
File: CleverTapSDK/Encryption/NetworkEncryptionManager.swift:15-28
Timestamp: 2025-06-03T16:29:34.418Z
Learning: In CleverTap iOS SDK, encryption in transit (PR #438) intentionally generates new session keys on each app launch for forward secrecy, which is different from the persistent key approach used for data at rest encryption (PR #401). The NetworkEncryptionManager is designed for temporary encryption of network communications to the backend, not for persistent data storage.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.m:0-0
Timestamp: 2025-04-17T10:49:43.627Z
Learning: In the CleverTap iOS SDK, the previous AES-CBC implementation in CTAESCrypt.m was intentionally kept untouched (despite security issues like static IVs) during the encryption upgrade to AES-GCM to facilitate the migration of existing encrypted data.
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-15T05:02:57.045Z
Learning: CleverTap iOS SDK uses both AES-CBC (old) and AES-GCM (new) implementations during the encryption algorithm upgrade, with the older implementation maintained specifically for migration purposes.
📚 Learning: 2025-04-17T11:19:10.966Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTEncryptionManager.m:310-347
Timestamp: 2025-04-17T11:19:10.966Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), the implementation in CTEncryptionManager.m maintains the old AES-CBC implementation with static IV alongside the new AES-GCM implementation to support migration of existing encrypted data.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
📚 Learning: 2025-04-17T10:28:14.900Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401, replaced by `CTAESGCMCrypt.swift` which implements more secure key management using the Keychain.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
📚 Learning: 2025-04-17T10:34:46.988Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:34:46.988Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` previously used CBC mode without integrity verification, but has been completely replaced with `CTAESGCMCrypt.swift` using AES-GCM (authenticated encryption) in PR #401 to properly address data integrity concerns.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
📚 Learning: 2025-04-17T10:51:39.897Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTCryptHandler.m:0-0
Timestamp: 2025-04-17T10:51:39.897Z
Learning: The file `CleverTapSDK/Encryption/CTCryptHandler.m` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
📚 Learning: 2025-04-18T04:39:04.876Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTEncryptionManager.m:12-16
Timestamp: 2025-04-18T04:39:04.876Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), the hardcoded cryptographic constants in CTEncryptionManager.m (kCRYPT_KEY_PREFIX, kCRYPT_KEY_SUFFIX) are part of the legacy AES-CBC encryption that is being maintained only for migration purposes as the SDK transitions to AES-GCM encryption.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
📚 Learning: 2025-04-17T10:49:43.627Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.m:0-0
Timestamp: 2025-04-17T10:49:43.627Z
Learning: In the CleverTap iOS SDK, the previous AES-CBC implementation in CTAESCrypt.m was intentionally kept untouched (despite security issues like static IVs) during the encryption upgrade to AES-GCM to facilitate the migration of existing encrypted data.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
📚 Learning: 2025-04-17T10:28:14.900Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-17T10:28:14.900Z
Learning: The file `CleverTapSDK/Encryption/CTAESCrypt.swift` mentioned in a review was removed as part of the encryption algorithm upgrade to AES-GCM in PR #401.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
📚 Learning: 2025-04-17T11:28:19.496Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/CTLocalDataStore.m:0-0
Timestamp: 2025-04-17T11:28:19.496Z
Learning: In the CleverTap iOS SDK encryption upgrade (PR #401), error handling has been implemented for encryption/decryption operations to handle potential failures when working with PII data.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
📚 Learning: 2025-06-03T16:29:34.418Z
Learnt from: akashvercetti
PR: CleverTap/clevertap-ios-sdk#438
File: CleverTapSDK/Encryption/NetworkEncryptionManager.swift:15-28
Timestamp: 2025-06-03T16:29:34.418Z
Learning: In CleverTap iOS SDK, encryption in transit (PR #438) intentionally generates new session keys on each app launch for forward secrecy, which is different from the persistent key approach used for data at rest encryption (PR #401). The NetworkEncryptionManager is designed for temporary encryption of network communications to the backend, not for persistent data storage.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
  • CleverTapSDK/Inbox/controllers/CTInboxController.m
📚 Learning: 2025-04-17T10:30:45.773Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTKeychainManager.swift:0-0
Timestamp: 2025-04-17T10:30:45.773Z
Learning: The file CTKeychainManager.swift was removed in PR #401 and its keychain operations functionality was consolidated into CTAESGCMCrypt.swift as part of the encryption algorithm upgrade to AES-GCM, which provides improved key management using the Keychain.

Applied to files:

  • CleverTapSDK/ProductExperiences/CTVarCache.m
📚 Learning: 2025-06-26T07:29:08.854Z
Learnt from: Sonal-Kachare
PR: CleverTap/clevertap-ios-sdk#447
File: SwiftUIStarter/SwiftUIStarter/CTAppInboxCustomView.swift:34-34
Timestamp: 2025-06-26T07:29:08.854Z
Learning: In SwiftUI CleverTap integration, when InboxMessage objects are created from CleverTapInboxMessage (the SDK's own message type), the message IDs are guaranteed to be valid and non-empty, so additional guard checks before calling CleverTap SDK methods are not required.

Applied to files:

  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
📚 Learning: 2025-06-26T07:30:49.414Z
Learnt from: Sonal-Kachare
PR: CleverTap/clevertap-ios-sdk#447
File: SwiftStarter/SwiftStarter/Supporting Files/AppInboxTableViewCell.swift:34-34
Timestamp: 2025-06-26T07:30:49.414Z
Learning: In CleverTap iOS SDK integrations (both SwiftUI and UIKit), when objects are created from CleverTapInboxMessage (the SDK's own message type), the message IDs are guaranteed to be valid and non-empty, so additional guard checks before calling CleverTap SDK methods like recordInboxNotificationClickedEvent, recordInboxNotificationViewedEvent, markReadInboxMessage, and deleteInboxMessage are not required.

Applied to files:

  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
📚 Learning: 2025-06-03T16:25:54.721Z
Learnt from: akashvercetti
PR: CleverTap/clevertap-ios-sdk#438
File: CleverTapSDK/Encryption/NetworkEncryptionManager.swift:0-0
Timestamp: 2025-06-03T16:25:54.721Z
Learning: In the CleverTap iOS SDK NetworkEncryptionManager.swift, the encrypt() and decrypt() methods return empty values on failure rather than using Result types or throwing errors due to Objective-C interoperability constraints, as Swift Result types don't bridge cleanly to Objective-C.

Applied to files:

  • CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m
📚 Learning: 2025-04-15T05:02:57.045Z
Learnt from: kushCT
PR: CleverTap/clevertap-ios-sdk#401
File: CleverTapSDK/Encryption/CTAESCrypt.swift:0-0
Timestamp: 2025-04-15T05:02:57.045Z
Learning: CleverTap iOS SDK uses both AES-CBC (old) and AES-GCM (new) implementations during the encryption algorithm upgrade, with the older implementation maintained specifically for migration purposes.

Applied to files:

  • CleverTapSDK/Inbox/controllers/CTInboxController.m
🔇 Additional comments (8)
CleverTapSDK/ProductExperiences/CTVarCache.m (3)

6-6: Import is appropriate.

Bringing CTEncryptionManager into scope here makes sense for local encryption/decryption.


222-224: Encrypt-before-save flow looks good.

Assuming the decryption path is updated (see Lines 413–431), this preserves round‑trip.


230-230: Archiving key usage is correct.

Matches the unarchive key; no issues.

CleverTapSDK/Inbox/controllers/CTInboxController.m (5)

17-20: Added encryption fields on controller: LGTM

State is explicit and aligns with PR goals.


97-103: Pre-encrypt flow: LGTM

Encrypt-on-ingest keeps storage opaque while preserving lookup fields.


320-329: Bulk migration driver: LGTM

Iterates and delegates per-message work with clear logs.


362-368: Encryption detector helper: LGTM

Simple and centralized. Consider null-manager note in callers (addressed above).


33-53: Verify all CTInboxController call sites updated to the new initializer signature

Ensure every instantiation supplies encryptionLevel, previousEncryptionLevel, and encryptionManager. Sandbox ripgrep produced no hits — confirm locally and update any missing call sites.

Comment on lines 60 to 74
if ([encryptedString hasPrefix:AES_GCM_PREFIX] && [encryptedString hasSuffix:AES_GCM_SUFFIX]) {
// Get encryption manager from context
CTEncryptionManager *encryptionManager = self.user.encryptionManager;
if (encryptionManager) {
id decryptedObj = [encryptionManager decryptObject:encryptedString];
if (decryptedObj && [decryptedObj isKindOfClass:[NSDictionary class]]) {
jsonData = decryptedObj;
}
else {
CleverTapLogStaticDebug(@"Failed to decrypt message with ID: %@, returning empty dictionary", self.id);
return @{@"isRead": @(self.isRead), @"date": @(self.date)};
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Crash risk: json may remain an NSString; add safe fallback and prefer manager’s detector

If encryptionManager is nil or markers don’t match, jsonData stays an NSString and dictionaryWithDictionary: will crash. Also, prefer isTextAESGCMEncrypted: for detection.

Apply:

-        // Check if it's actually encrypted using AES-GCM markers
-        if ([encryptedString hasPrefix:AES_GCM_PREFIX] && [encryptedString hasSuffix:AES_GCM_SUFFIX]) {
-            // Get encryption manager from context
-            CTEncryptionManager *encryptionManager =  self.user.encryptionManager;
-            if (encryptionManager) {
+        // Use manager’s detector when available
+        CTEncryptionManager *encryptionManager = self.user.encryptionManager;
+        if (encryptionManager && [encryptionManager isTextAESGCMEncrypted:encryptedString]) {
             id decryptedObj = [encryptionManager decryptObject:encryptedString];
             if (decryptedObj && [decryptedObj isKindOfClass:[NSDictionary class]]) {
                 jsonData = decryptedObj;
             }
             else {
                 CleverTapLogStaticDebug(@"Failed to decrypt message with ID: %@, returning empty dictionary", self.id);
                 return @{@"isRead": @(self.isRead), @"date": @(self.date)};
             }
+        } else if ([jsonData isKindOfClass:[NSString class]]) {
+            CleverTapLogStaticDebug(@"No encryption manager or unrecognized encrypted payload for message ID: %@, returning empty dictionary", self.id);
+            return @{@"isRead": @(self.isRead), @"date": @(self.date)};
         }
🤖 Prompt for AI Agents
In CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m around lines 60 to
74, the code assumes jsonData becomes an NSDictionary after decryption which can
crash if encryptionManager is nil or markers don't match; replace the
prefix/suffix check with the manager method isTextAESGCMEncrypted: (prefer
manager detection), attempt decryption only when that returns YES and
encryptionManager is non-nil, and after decryption add a safe fallback: if
jsonData is still an NSString (or decryption failed/not NSDictionary), do not
call dictionaryWithDictionary: — instead return a safe default NSDictionary
(e.g., only include isRead and date) or attempt to parse the string into JSON
safely before casting to NSDictionary. Ensure all paths where decryption is
expected but unavailable return the safe fallback to avoid crashes.

Comment on lines +76 to 79
NSMutableDictionary *json = [NSMutableDictionary dictionaryWithDictionary:jsonData];
json[@"isRead"] = @(self.isRead);
json[@"date"] = @(self.date);
return json;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Defensive type check before building the JSON dictionary

Ensure jsonData is NSDictionary; otherwise return a safe minimal payload.

Apply:

-    NSMutableDictionary *json = [NSMutableDictionary dictionaryWithDictionary:jsonData];
+    if (![jsonData isKindOfClass:[NSDictionary class]]) {
+        CleverTapLogStaticDebug(@"Invalid inbox message JSON type for ID: %@, returning empty dictionary", self.id);
+        return @{@"isRead": @(self.isRead), @"date": @(self.date)};
+    }
+    NSMutableDictionary *json = [NSMutableDictionary dictionaryWithDictionary:(NSDictionary *)jsonData];
📝 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.

Suggested change
NSMutableDictionary *json = [NSMutableDictionary dictionaryWithDictionary:jsonData];
json[@"isRead"] = @(self.isRead);
json[@"date"] = @(self.date);
return json;
if (![jsonData isKindOfClass:[NSDictionary class]]) {
CleverTapLogStaticDebug(@"Invalid inbox message JSON type for ID: %@, returning empty dictionary", self.id);
return @{@"isRead": @(self.isRead), @"date": @(self.date)};
}
NSMutableDictionary *json = [NSMutableDictionary dictionaryWithDictionary:(NSDictionary *)jsonData];
json[@"isRead"] = @(self.isRead);
json[@"date"] = @(self.date);
return json;
🤖 Prompt for AI Agents
In CleverTapSDK/Inbox/models/CTMessageMO+CoreDataProperties.m around lines 76 to
79, the code assumes jsonData is an NSDictionary before building json; add a
defensive type check and if jsonData is not an NSDictionary create and return a
safe minimal NSDictionary payload (for example a NSMutableDictionary containing
at least @"isRead" with @(self.isRead) and @"date" with @(self.date) or sensible
defaults), otherwise proceed to copy jsonData into a mutable dictionary and
add/overwrite the isRead and date keys as currently done.

Comment on lines +18 to +19
@property (nonatomic, assign) CleverTapEncryptionLevel encryptionLevel;
@property (nonatomic, assign) CleverTapEncryptionLevel previousEncryptionLevel;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid duplicating encryption level state; risk of drift vs config.

Mirroring levels on this class can become stale if config.encryptionLevel or config.cryptManager.previousEncryptionLevel changes post-init. Prefer deriving “current” and “previous” from config at save-time and persisting back to cryptManager.

Apply this diff in saveDiffs (see lines 212–217) to make config the source of truth and persist the new previous level:

-    // Handle encryption level migration if needed
-    if (self.encryptionLevel != self.previousEncryptionLevel) {
-        [self migrateVariablesEncryption];
-        self.previousEncryptionLevel = self.encryptionLevel; // Update to prevent repeated migration
-    }
+    // Handle encryption level migration if needed (config is source of truth)
+    CleverTapEncryptionLevel currentLevel = self.config.encryptionLevel;
+    CleverTapEncryptionLevel previousLevel = self.config.cryptManager.previousEncryptionLevel;
+    if (currentLevel != previousLevel) {
+        self.previousEncryptionLevel = previousLevel;
+        self.encryptionLevel = currentLevel;
+        [self migrateVariablesEncryption];
+        // Persist the migrated level so we don't re-migrate next launch
+        self.config.cryptManager.previousEncryptionLevel = currentLevel;
+        self.previousEncryptionLevel = currentLevel;
+    }
🤖 Prompt for AI Agents
In CleverTapSDK/ProductExperiences/CTVarCache.m around lines 18–19 and in
saveDiffs around lines 212–217, remove reliance on the duplicated
encryptionLevel and previousEncryptionLevel properties and instead read the
current level from self.config.encryptionLevel and the previous level from
self.config.cryptManager.previousEncryptionLevel when preparing diffs; when
updating, set self.config.cryptManager.previousEncryptionLevel to the prior
current level and persist the config (or persist via cryptManager) so the new
previous level is stored back to the config/cryptManager rather than keeping
stale state on this class.

Comment on lines +212 to +217
// Handle encryption level migration if needed
if (self.encryptionLevel != self.previousEncryptionLevel) {
[self migrateVariablesEncryption];
self.previousEncryptionLevel = self.encryptionLevel; // Update to prevent repeated migration
}

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Migration gating uses potentially stale properties and doesn’t persist new level.

If config.encryptionLevel changes during runtime, this block may never run. Also, only the local previousEncryptionLevel is updated, not config.cryptManager.previousEncryptionLevel, so migration can repeat next launch.

Use the diff provided in Lines 18–19 to fix both issues.

Comment on lines +413 to +431
- (NSDictionary *)decryptDiffsIfNeeded:(NSDictionary *)diffs {
if (!diffs || [diffs count] == 0) {
return diffs;
}

// Check if this is encrypted data
if (diffs[@"_ct_encrypted_vars"]) {
NSString *encryptedString = diffs[@"_ct_encrypted_vars"];
if ([self.config.cryptManager isTextAESGCMEncrypted:encryptedString]) {
id decryptedDiffs = [self.config.cryptManager decryptObject:encryptedString];
if (decryptedDiffs && [decryptedDiffs isKindOfClass:[NSDictionary class]]) {
CleverTapLogDebug(self.config.logLevel, @"%@: Decrypted variable diffs from storage", self);
return decryptedDiffs;
}
}
}

return diffs;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Don’t gate decryption strictly on AES‑GCM; attempt decrypt for migration.

Per prior upgrades, CTEncryptionManager retains legacy AES‑CBC decryption for migration. If you check isTextAESGCMEncrypted first, CBC‑encrypted payloads won’t be decrypted and the sentinel dict leaks into merging.

Apply:

-    // Check if this is encrypted data
-    if (diffs[@"_ct_encrypted_vars"]) {
-        NSString *encryptedString = diffs[@"_ct_encrypted_vars"];
-        if ([self.config.cryptManager isTextAESGCMEncrypted:encryptedString]) {
-            id decryptedDiffs = [self.config.cryptManager decryptObject:encryptedString];
-            if (decryptedDiffs && [decryptedDiffs isKindOfClass:[NSDictionary class]]) {
-                CleverTapLogDebug(self.config.logLevel, @"%@: Decrypted variable diffs from storage", self);
-                return decryptedDiffs;
-            }
-        }
-    }
+    // Check if this is encrypted data
+    if (diffs[@"_ct_encrypted_vars"]) {
+        NSString *encryptedString = diffs[@"_ct_encrypted_vars"];
+        id decryptedDiffs = [self.config.cryptManager decryptObject:encryptedString]; // handles AES‑GCM and legacy CBC
+        if ([decryptedDiffs isKindOfClass:[NSDictionary class]]) {
+            CleverTapLogDebug(self.config.logLevel, @"%@: Decrypted variable diffs from storage", self);
+            return (NSDictionary *)decryptedDiffs;
+        } else {
+            CleverTapLogStaticInternal(@"%@: Failed to decrypt variable diffs; leaving as-is", self);
+        }
+    }
📝 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.

Suggested change
- (NSDictionary *)decryptDiffsIfNeeded:(NSDictionary *)diffs {
if (!diffs || [diffs count] == 0) {
return diffs;
}
// Check if this is encrypted data
if (diffs[@"_ct_encrypted_vars"]) {
NSString *encryptedString = diffs[@"_ct_encrypted_vars"];
if ([self.config.cryptManager isTextAESGCMEncrypted:encryptedString]) {
id decryptedDiffs = [self.config.cryptManager decryptObject:encryptedString];
if (decryptedDiffs && [decryptedDiffs isKindOfClass:[NSDictionary class]]) {
CleverTapLogDebug(self.config.logLevel, @"%@: Decrypted variable diffs from storage", self);
return decryptedDiffs;
}
}
}
return diffs;
}
- (NSDictionary *)decryptDiffsIfNeeded:(NSDictionary *)diffs {
if (!diffs || [diffs count] == 0) {
return diffs;
}
// Check if this is encrypted data
if (diffs[@"_ct_encrypted_vars"]) {
NSString *encryptedString = diffs[@"_ct_encrypted_vars"];
id decryptedDiffs = [self.config.cryptManager decryptObject:encryptedString]; // handles AES-GCM and legacy CBC
if ([decryptedDiffs isKindOfClass:[NSDictionary class]]) {
CleverTapLogDebug(self.config.logLevel, @"%@: Decrypted variable diffs from storage", self);
return (NSDictionary *)decryptedDiffs;
} else {
CleverTapLogStaticInternal(@"%@: Failed to decrypt variable diffs; leaving as-is", self);
}
}
return diffs;
}

@akashvercetti akashvercetti force-pushed the SDK-5154-encryption-high branch from 8ae7886 to ca87908 Compare September 17, 2025 07:02
@akashvercetti akashvercetti marked this pull request as ready for review October 6, 2025 06:57
cursor[bot]

This comment was marked as outdated.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants