sync: support batched mode for sync state and handle clean sync (#944)

Support batched mode for sync state and handle clean sync

Fixes SNT-357

---------

Co-authored-by: Matt W <436037+mlw@users.noreply.github.com>

Author: sharvilshah

Source/common/SNTConfigBundle.h

@@ -56,4 +56,11 @@
 - (void)fullSyncInterval:(void (^)(NSUInteger))block;
 - (void)pushNotificationsFullSyncInterval:(void (^)(NSUInteger))block;
 
+///
+///  When set, signals the daemon to clear persisted sync state before
+///  applying the rest of the bundle. Set only by PostflightConfigBundle
+///  when the in-flight sync was Clean or CleanAll.
+///
+- (void)clearSyncStateBeforeApply:(void (^)(BOOL))block;
+
 @end

Source/common/SNTConfigBundle.mm

@@ -52,6 +52,7 @@ @interface SNTConfigBundle ()
 @property NSArray<SNTCELFallbackRule*>* celFallbackRules;
 @property NSNumber* fullSyncInterval;
 @property NSNumber* pushNotificationsFullSyncInterval;
+@property NSNumber* clearSyncStateBeforeApply;
 @end
 
 @implementation SNTConfigBundle
@@ -92,6 +93,7 @@ - (void)encodeWithCoder:(NSCoder*)coder {
   ENCODE(coder, celFallbackRules);
   ENCODE(coder, fullSyncInterval);
   ENCODE(coder, pushNotificationsFullSyncInterval);
+  ENCODE(coder, clearSyncStateBeforeApply);
 }
 
 - (instancetype)initWithCoder:(NSCoder*)decoder {
@@ -128,6 +130,7 @@ - (instancetype)initWithCoder:(NSCoder*)decoder {
     DECODE_ARRAY(decoder, celFallbackRules, SNTCELFallbackRule);
     DECODE(decoder, fullSyncInterval, NSNumber);
     DECODE(decoder, pushNotificationsFullSyncInterval, NSNumber);
+    DECODE(decoder, clearSyncStateBeforeApply, NSNumber);
   }
   return self;
 }
@@ -317,4 +320,10 @@ - (void)pushNotificationsFullSyncInterval:(void (^)(NSUInteger))block {
   }
 }
 
+- (void)clearSyncStateBeforeApply:(void (^)(BOOL))block {
+  if (self.clearSyncStateBeforeApply) {
+    block([self.clearSyncStateBeforeApply boolValue]);
+  }
+}
+
 @end

Source/common/SNTConfigBundleTest.mm

@@ -53,6 +53,7 @@ @interface SNTConfigBundle (Testing)
 @property NSArray<SNTCELFallbackRule*>* celFallbackRules;
 @property NSNumber* fullSyncInterval;
 @property NSNumber* pushNotificationsFullSyncInterval;
+@property NSNumber* clearSyncStateBeforeApply;
 @end
 
 @interface SNTConfigBundleTest : XCTestCase
@@ -372,4 +373,46 @@ - (void)testGettersWithoutValues {
   }];
 }
 
+- (void)testClearSyncStateBeforeApplyRoundTrips {
+  // When set to YES, the round-trip preserves the value and the accessor block fires with YES.
+  SNTConfigBundle* set = [[SNTConfigBundle alloc] init];
+  set.clearSyncStateBeforeApply = @(YES);
+
+  NSError* error = nil;
+  NSData* setData = [NSKeyedArchiver archivedDataWithRootObject:set
+                                          requiringSecureCoding:YES
+                                                          error:&error];
+  XCTAssertNil(error);
+  SNTConfigBundle* setDecoded = [NSKeyedUnarchiver unarchivedObjectOfClass:[SNTConfigBundle class]
+                                                                  fromData:setData
+                                                                     error:&error];
+  XCTAssertNil(error);
+
+  __block BOOL setFired = NO;
+  __block BOOL setValue = NO;
+  [setDecoded clearSyncStateBeforeApply:^(BOOL v) {
+    setFired = YES;
+    setValue = v;
+  }];
+  XCTAssertTrue(setFired);
+  XCTAssertEqual(setValue, YES);
+
+  // When left unset, the round-trip preserves the unset state and the accessor block does not fire.
+  SNTConfigBundle* unset = [[SNTConfigBundle alloc] init];
+  NSData* unsetData = [NSKeyedArchiver archivedDataWithRootObject:unset
+                                            requiringSecureCoding:YES
+                                                            error:&error];
+  XCTAssertNil(error);
+  SNTConfigBundle* unsetDecoded = [NSKeyedUnarchiver unarchivedObjectOfClass:[SNTConfigBundle class]
+                                                                    fromData:unsetData
+                                                                       error:&error];
+  XCTAssertNil(error);
+
+  __block BOOL unsetFired = NO;
+  [unsetDecoded clearSyncStateBeforeApply:^(BOOL v) {
+    unsetFired = YES;
+  }];
+  XCTAssertFalse(unsetFired);
+}
+
 @end

Source/common/SNTConfigurator.h

@@ -1152,6 +1152,24 @@ extern NSString* _Nonnull const kEnableMenuItemUserOverride;
 ///
 - (void)clearSyncState;
 
+///
+///  Buffer a series of sync-state mutations into a single atomic commit.
+///
+///  Inside the block, calls to `updateSyncStateForKey:value:` and
+///  `clearSyncState` write only to an in-memory working dictionary —
+///  they do not fire KVO and do not write to disk. When the block
+///  returns, the working dictionary is assigned to `syncState` (single
+///  KVO fire) and saved to disk (single `writeToFile:atomically:YES`).
+///
+///  Returns `YES` when both the in-memory commit and the disk write
+///  succeed. Returns `NO` when nested inside another batch (asserts in
+///  debug, logs and skips in release) or when the disk write fails. The
+///  block is not expected to raise exceptions; if one escapes it will
+///  crash the daemon, by design. Callers should gate any post-commit
+///  work that depends on durable state on the return value.
+///
+- (BOOL)performSyncStateBatch:(nonnull void(NS_NOESCAPE ^)(void))block;
+
 ///
 ///  Validate the configuration profile.
 ///

Source/common/SNTConfigurator.mm

@@ -82,6 +82,11 @@ @interface SNTConfigurator ()
 @property(atomic) NSMutableDictionary* configState;
 @property(atomic) NSDictionary* state;
 
+/// Working dictionary for an in-progress `performSyncStateBatch:` transaction.
+/// Non-nil only while inside the batch's block. Always accessed on the main
+/// thread, matching the convention enforced by `updateSyncStateForKey:value:`.
+@property(nonatomic) NSMutableDictionary* batchedSyncState;
+
 @property(readonly, nonatomic) NSString* syncStateFilePath;
 @property(readonly, nonatomic) NSString* stateFilePath;
 
@@ -1960,6 +1965,10 @@ - (NSDictionary*)extraMetricLabels {
 ///
 - (void)updateSyncStateForKey:(NSString*)key value:(id)value {
   void (^block)(void) = ^{
+    if (self.batchedSyncState) {
+      self.batchedSyncState[key] = value;
+      return;
+    }
     NSMutableDictionary* syncState = self.syncState.mutableCopy;
     syncState[key] = value;
     self.syncState = syncState;
@@ -1974,6 +1983,28 @@ - (void)updateSyncStateForKey:(NSString*)key value:(id)value {
   }
 }
 
+- (BOOL)performSyncStateBatch:(void(NS_NOESCAPE ^)(void))block {
+  __block BOOL committed = NO;
+  void (^run)(void) = ^{
+    if (self.batchedSyncState != nil) {
+      NSAssert(NO, @"Sync-state batches do not nest");
+      LOGE(@"Sync-state batches do not nest; skipping nested batch");
+      return;
+    }
+    self.batchedSyncState = self.syncState.mutableCopy;
+    block();
+    self.syncState = self.batchedSyncState;
+    self.batchedSyncState = nil;
+    committed = [self saveSyncStateToDisk];
+  };
+  if ([NSThread isMainThread]) {
+    run();
+  } else {
+    dispatch_sync(dispatch_get_main_queue(), run);
+  }
+  return committed;
+}
+
 ///
 ///  Read the saved syncState.
 ///
@@ -2043,27 +2074,47 @@ - (BOOL)migrateDeprecatedSyncStateKeys {
 }
 
 ///
-///  Saves the current effective syncState to disk.
+///  Saves the current effective syncState to disk. Returns YES if the write
+///  succeeded; NO if the authorizer denied the operation or the underlying
+///  file write failed.
 ///
-- (void)saveSyncStateToDisk {
+- (BOOL)saveSyncStateToDisk {
   if (!self.syncStateAccessAuthorizerBlock()) {
-    return;
+    return NO;
   }
 
   NSMutableDictionary* syncState = self.syncState.mutableCopy;
   syncState[kAllowedPathRegexKey] = [syncState[kAllowedPathRegexKey] pattern];
   syncState[kBlockedPathRegexKey] = [syncState[kBlockedPathRegexKey] pattern];
-  [syncState writeToFile:self.syncStateFilePath atomically:YES];
+  if (![syncState writeToFile:self.syncStateFilePath atomically:YES]) {
+    LOGE(@"Failed to write sync state to %@", self.syncStateFilePath);
+    return NO;
+  }
   [[NSFileManager defaultManager] setAttributes:@{NSFilePosixPermissions : @0600}
                                    ofItemAtPath:self.syncStateFilePath
                                           error:NULL];
+  return YES;
 }
 
 - (void)clearSyncState {
-  self.syncState = [NSMutableDictionary dictionary];
-  // TODO: Start a timer to flush the state to disk. On startup, Santa should
-  // check for the presence of the state file and, if no SyncBaseURL is
-  // configured, start the timer to clear sync state and flush to disk.
+  // Intentionally not gated on `syncStateAccessAuthorizerBlock`: the authorizer
+  // requires `syncBaseURL != nil`, but the SNTSyncdQueue caller invokes this
+  // method precisely *because* `syncBaseURL` went to nil. Gating here would
+  // make the cleanup unreachable in production. Disk removal still requires
+  // root, which santad already has.
+  void (^block)(void) = ^{
+    if (self.batchedSyncState) {
+      [self.batchedSyncState removeAllObjects];
+      return;
+    }
+    self.syncState = [NSMutableDictionary dictionary];
+    [[NSFileManager defaultManager] removeItemAtPath:self.syncStateFilePath error:NULL];
+  };
+  if ([NSThread isMainThread]) {
+    block();
+  } else {
+    dispatch_sync(dispatch_get_main_queue(), block);
+  }
 }
 
 - (NSArray*)entitlementsPrefixFilter {