sync: emit events for USB mount blocks #752
Conversation
github-actions
bot
added
comp/santad
📝 WalkthroughWalkthroughThis PR introduces USB mount event emission by creating a new SNTStoredUSBMountEvent model class, integrating it into the device manager to capture USB mount details, and extending the sync service to upload these events to servers. Changes
Sequence DiagramsequenceDiagram
actor Device as Device (USB Mount)
participant DevMgr as Device Manager
participant Santad as Santad Service
participant Queue as Event Queue
participant SyncService as Sync Service
participant Server as Sync Server
Device->>DevMgr: USB device detected
DevMgr->>DevMgr: Create SNTStoredUSBMountEvent<br/>(uuid, model, vendor, mountName)
DevMgr->>Santad: usbMountCallback(event)
Santad->>Queue: addStoredEvent(event)
Note over Queue: Event stored locally
Queue->>SyncService: Event retrieval
SyncService->>SyncService: Create USBMountEvent<br/>MessageForUSBMountEvent()
SyncService->>SyncService: Populate batch with<br/>USB mount events
SyncService->>Server: Upload event batch (V2)
Server->>Server: Process USB mount event
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🤖 Fix all issues with AI agents
In `@Source/common/SNTStoredUSBMountEventTest.mm`:
- Around line 65-69: The secure unarchive call only whitelists
SNTStoredUSBMountEvent but must include every concrete class in the object
graph; update the allowedClasses NSSet used with unarchivedObjectOfClasses to
also include NSString, NSNumber, and NSDate (and any other concrete classes used
by SNTStoredUSBMountEvent or its superclass SNTStoredEvent) so secure decoding
succeeds; ensure both occurrences (the unarchive at places referencing
SNTStoredUSBMountEvent, e.g., the block using unarchivedObjectOfClasses and the
similar block later around the other test) are updated to include these classes.
🧹 Nitpick comments (1)
Source/common/SNTStoredUSBMountEvent.mm (1)
62-65: Consider a UUID fallback whenmountOnNameis missing.
Returning"(null)"can collapse dedupe keys ifmountOnNameis nil; a UUID fallback avoids silent coalescing.♻️ Suggested fallback to avoid dedupe collisions
- return [NSString stringWithFormat:@"%@", self.mountOnName]; + if (self.mountOnName.length > 0) { + return self.mountOnName; + } + return self.uuid ?: @"";
| // Unarchive the event | ||
| NSSet *allowedClasses = [NSSet setWithObject:[SNTStoredUSBMountEvent class]]; | ||
| SNTStoredEvent *unarchivedEvent = [NSKeyedUnarchiver unarchivedObjectOfClasses:allowedClasses | ||
| fromData:archivedEvent | ||
| error:nil]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the relevant files
find . -name "SNTStoredUSBMountEvent*" -type fRepository: northpolesec/santa
Length of output: 190
🏁 Script executed:
# Get the test file content around the mentioned lines
cat -n Source/common/SNTStoredUSBMountEventTest.mm | sed -n '60,100p'Repository: northpolesec/santa
Length of output: 2312
🏁 Script executed:
# Find SNTStoredUSBMountEvent implementation
find . -name "SNTStoredUSBMountEvent.h" -o -name "SNTStoredUSBMountEvent.m" | head -5Repository: northpolesec/santa
Length of output: 102
🏁 Script executed:
# Read SNTStoredUSBMountEvent header
cat -n Source/common/SNTStoredUSBMountEvent.hRepository: northpolesec/santa
Length of output: 1416
🏁 Script executed:
# Read SNTStoredUSBMountEvent implementation
cat -n Source/common/SNTStoredUSBMountEvent.mmRepository: northpolesec/santa
Length of output: 2336
🏁 Script executed:
# Check CoderMacros.h to understand the DECODE macro
find . -name "CoderMacros.h" -exec cat -n {} \;Repository: northpolesec/santa
Length of output: 4886
🏁 Script executed:
# Check if SNTStoredEvent (the parent class) also has NSString properties
find . -name "SNTStoredEvent.h" -exec cat -n {} \;Repository: northpolesec/santa
Length of output: 1650
🏁 Script executed:
# Check the SNTStoredEvent implementation to see its encoding/decoding
find . -name "SNTStoredEvent.mm" -o -name "SNTStoredEvent.m" | head -1 | xargs cat -n 2>/dev/null || echo "No implementation found"Repository: northpolesec/santa
Length of output: 2339
🌐 Web query:
NSKeyedUnarchiver unarchivedObjectOfClasses NSString allowedClasses secure coding
💡 Result:
When you use NSKeyedUnarchiver.unarchivedObject(ofClasses:from:) / +unarchivedObjectOfClasses:fromData:error:, the allowed-classes set must include every concrete class that might be decoded anywhere in the object graph, not just the top-level container. If it doesn’t, you’ll see errors like “value for key NS.objects was of unexpected class …” or warnings that a “safe plist type” like NSString was decoded even though it wasn’t listed (and that this “will be disallowed in the future”). [1], [2], [3]
Typical fix (NSString missing from allowedClasses)
Swift
let allowed: [AnyClass] = [NSArray.self, NSString.self] // plus whatever else is inside
let obj = try NSKeyedUnarchiver.unarchivedObject(ofClasses: Set(allowed), from: data)[1], [2]
Objective-C
NSSet *allowed = [NSSet setWithObjects:NSArray.class, NSString.class, nil];
NSError *error = nil;
id obj = [NSKeyedUnarchiver unarchivedObjectOfClasses:allowed fromData:data error:&error];[1], [2]
What to put in allowedClasses
Include:
- The root class you expect back (e.g.,
NSArray,NSDictionary, your model class) - Any nested container classes (
NSArray,NSDictionary,NSSet, etc.) - Any plist/scalar object classes that appear inside (
NSString,NSNumber,NSData,NSDate, etc.) - Any custom classes stored inside the containers
Apple’s forums specifically call out that if you decode an NSArray that contains custom objects, you must use unarchivedObjectOfClasses: and include the contained classes too (and this applies recursively for nested collections). [3]
Avoid “broad” classes like [NSObject class] as an allowed class; Foundation warns this bypasses the security model and may become an error. [4]
Sources: Apple method doc page (signature/availability) [1]; iOS 15+ “safe plist type ‘NSString’ missing from allowed classes” warning discussion [2]; Apple Developer Forums explanation that you must include contained classes for collections [3]; Foundation warning about allowing NSObject (security bypass) [4].
Add all required classes to allowedClasses for secure unarchiving.
The unarchivedObjectOfClasses method requires every concrete class in the entire object graph to be whitelisted. The current code only allows SNTStoredUSBMountEvent, but decoding will attempt to instantiate NSString (from the four NSString properties), NSNumber (from the parent's idx), and NSDate (from the parent's occurrenceDate). All must be included in allowedClasses or secure decoding will fail.
Proposed fix
- NSSet *allowedClasses = [NSSet setWithObject:[SNTStoredUSBMountEvent class]];
+ NSSet *allowedClasses = [NSSet setWithObjects:[SNTStoredUSBMountEvent class],
+ [NSString class], [NSNumber class], [NSDate class], nil];Also applies to: 95-98
🤖 Prompt for AI Agents
In `@Source/common/SNTStoredUSBMountEventTest.mm` around lines 65 - 69, The secure
unarchive call only whitelists SNTStoredUSBMountEvent but must include every
concrete class in the object graph; update the allowedClasses NSSet used with
unarchivedObjectOfClasses to also include NSString, NSNumber, and NSDate (and
any other concrete classes used by SNTStoredUSBMountEvent or its superclass
SNTStoredEvent) so secure decoding succeeds; ensure both occurrences (the
unarchive at places referencing SNTStoredUSBMountEvent, e.g., the block using
unarchivedObjectOfClasses and the similar block later around the other test) are
updated to include these classes.
| /// limitations under the License. | ||
|
|
||
| #import "Source/common/SNTStoredUSBMountEvent.h" | ||
| #include <Foundation/Foundation.h> |
There was a problem hiding this comment.
| #include <Foundation/Foundation.h> | |
| #include <Foundation/Foundation.h> |
Or remove this import, it's not needed as it's already imported in the header.
| return [NSString stringWithFormat:@"SNTStoredUSBMountEvent[%@]: %@, By: %@", self.idx, | ||
| self.deviceModel, self.mountOnName]; |
There was a problem hiding this comment.
Extreme 55DD, By: /Volumes/laptop reads a bit odd. How about this?
| return [NSString stringWithFormat:@"SNTStoredUSBMountEvent[%@]: %@, By: %@", self.idx, | |
| self.deviceModel, self.mountOnName]; | |
| return [NSString stringWithFormat:@"SNTStoredUSBMountEvent[%@]: %@ %@ on: %@", self.idx, | |
| self.deviceVendor, self.deviceModel, self.mountOnName]; |
SNT-260
Test Plan:
This is how these events look look: