diff --git a/Source/common/SNTConfigBundle.h b/Source/common/SNTConfigBundle.h index c281cb1be..08eaf6c6d 100644 --- a/Source/common/SNTConfigBundle.h +++ b/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 diff --git a/Source/common/SNTConfigBundle.mm b/Source/common/SNTConfigBundle.mm index 8e0b9a8c3..774b86a3f 100644 --- a/Source/common/SNTConfigBundle.mm +++ b/Source/common/SNTConfigBundle.mm @@ -52,6 +52,7 @@ @interface SNTConfigBundle () @property NSArray* 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 diff --git a/Source/common/SNTConfigBundleTest.mm b/Source/common/SNTConfigBundleTest.mm index a217bb9cd..fc00a4c13 100644 --- a/Source/common/SNTConfigBundleTest.mm +++ b/Source/common/SNTConfigBundleTest.mm @@ -53,6 +53,7 @@ @interface SNTConfigBundle (Testing) @property NSArray* 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 diff --git a/Source/common/SNTConfigurator.h b/Source/common/SNTConfigurator.h index 77f2c69b2..49fc97c96 100644 --- a/Source/common/SNTConfigurator.h +++ b/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. /// diff --git a/Source/common/SNTConfigurator.mm b/Source/common/SNTConfigurator.mm index ed397282d..ebca32bb4 100644 --- a/Source/common/SNTConfigurator.mm +++ b/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 { diff --git a/Source/common/SNTConfiguratorTest.mm b/Source/common/SNTConfiguratorTest.mm index a95d63778..fe54b5309 100644 --- a/Source/common/SNTConfiguratorTest.mm +++ b/Source/common/SNTConfiguratorTest.mm @@ -285,4 +285,172 @@ - (void)testAllowDelegatedSignalsOverride { XCTAssertFalse(sut.allowDelegatedSignals); } +#pragma mark - performSyncStateBatch: and clearSyncState tests + +- (SNTConfigurator*)configuratorWithEmptySyncStateAtPath:(NSString*)plistPath { + return [[SNTConfigurator alloc] initWithSyncStateFile:plistPath + stateFile:@"/does/not/need/to/exist" + oldStateFile:@"/does/not/need/to/exist" + syncStateAccessAuthorizer:^{ + return YES; + } + stateAccessAuthorizer:^BOOL { + return NO; + }]; +} + +- (void)observeValueForKeyPath:(NSString*)keyPath + ofObject:(id)object + change:(NSDictionary*)change + context:(void*)context { + if (context != NULL) { + NSUInteger* count = (NSUInteger*)context; + (*count)++; + } +} + +- (void)testPerformSyncStateBatchCommitsAsSingleKVOFire { + NSString* plistPath = [NSString stringWithFormat:@"%@/batch-kvo.plist", self.testDir]; + SNTConfigurator* cfg = [self configuratorWithEmptySyncStateAtPath:plistPath]; + + __block NSUInteger kvoCount = 0; + [cfg addObserver:self + forKeyPath:@"syncState" + options:NSKeyValueObservingOptionNew + context:&kvoCount]; + + BOOL committed = [cfg performSyncStateBatch:^{ + [cfg setSyncServerClientMode:SNTClientModeLockdown]; + [cfg setEnableBundles:YES]; + [cfg setEnableTransitiveRules:YES]; + }]; + + [cfg removeObserver:self forKeyPath:@"syncState" context:&kvoCount]; + + XCTAssertTrue(committed, @"A successful batch must report committed=YES"); + XCTAssertEqual(kvoCount, (NSUInteger)1, + @"Expected exactly one KVO fire on syncState across the batch; got %lu", + (unsigned long)kvoCount); + XCTAssertEqual(cfg.clientMode, SNTClientModeLockdown); + XCTAssertTrue(cfg.enableBundles); + XCTAssertTrue(cfg.enableTransitiveRules); + + XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); +} + +- (void)testPerformSyncStateBatchReturnsNoWhenDiskWriteFails { + // Pointing the configurator at an unwritable path forces saveSyncStateToDisk + // to fail. The in-memory commit still happens (KVO still fires) but the + // BOOL return signals that durability was not achieved. + NSString* plistPath = @"/this/directory/definitely/does/not/exist/batch.plist"; + SNTConfigurator* cfg = [self configuratorWithEmptySyncStateAtPath:plistPath]; + + __block NSUInteger kvoCount = 0; + [cfg addObserver:self + forKeyPath:@"syncState" + options:NSKeyValueObservingOptionNew + context:&kvoCount]; + + BOOL committed = [cfg performSyncStateBatch:^{ + [cfg setSyncServerClientMode:SNTClientModeLockdown]; + }]; + + [cfg removeObserver:self forKeyPath:@"syncState" context:&kvoCount]; + + XCTAssertFalse(committed, @"Batch must report committed=NO when the disk write fails"); + XCTAssertEqual(kvoCount, (NSUInteger)1, + @"In-memory state still commits before the disk write is attempted"); + XCTAssertEqual(cfg.clientMode, SNTClientModeLockdown); +} + +- (void)testPerformSyncStateBatchWithClearAndWritesPersistsOnlyPostClearWrites { + NSString* plistPath = [NSString stringWithFormat:@"%@/batch-clear.plist", self.testDir]; + SNTConfigurator* cfg = [self configuratorWithEmptySyncStateAtPath:plistPath]; + + // Pre-populate stale state via the one-shot path. + [cfg setSyncServerClientMode:SNTClientModeLockdown]; + [cfg setEnableBundles:YES]; + XCTAssertEqual(cfg.clientMode, SNTClientModeLockdown); + XCTAssertTrue(cfg.enableBundles); + + // Inside the batch: clear, then write only one new key. + [cfg performSyncStateBatch:^{ + [cfg clearSyncState]; + [cfg setSyncServerClientMode:SNTClientModeMonitor]; + }]; + + // ClientMode set to the new value; EnableBundles was cleared and not rewritten. + XCTAssertEqual(cfg.clientMode, SNTClientModeMonitor); + XCTAssertFalse(cfg.enableBundles, @"EnableBundles should be cleared (default NO) after batch"); + + // On disk: matches in-memory state, no stale keys. + NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; + XCTAssertEqualObjects(onDisk[@"ClientMode"], @(SNTClientModeMonitor)); + XCTAssertNil(onDisk[@"EnableBundles"], @"Stale EnableBundles key should be absent from disk"); + + XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); +} + +- (void)testClearSyncStateRemovesDiskFileWhenOutsideBatch { + NSString* plistPath = [NSString stringWithFormat:@"%@/clear-remove.plist", self.testDir]; + SNTConfigurator* cfg = [self configuratorWithEmptySyncStateAtPath:plistPath]; + + // Force a disk write so the file exists. + [cfg setSyncServerClientMode:SNTClientModeLockdown]; + XCTAssertTrue([self.fileMgr fileExistsAtPath:plistPath]); + + [cfg clearSyncState]; + + XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath], + @"clearSyncState must remove sync-state.plist from disk"); + XCTAssertEqual(cfg.syncState.count, (NSUInteger)0); +} + +- (void)testClearSyncStateIsIdempotentOnMissingFile { + NSString* plistPath = [NSString stringWithFormat:@"%@/clear-missing.plist", self.testDir]; + SNTConfigurator* cfg = [self configuratorWithEmptySyncStateAtPath:plistPath]; + + XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]); + + // Should be a no-op, no exception, no error. + XCTAssertNoThrow([cfg clearSyncState]); + XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]); + + // A second call is also a no-op. + XCTAssertNoThrow([cfg clearSyncState]); +} + +- (void)testClearSyncStateBypassesSyncStateAccessAuthorizer { + // The production authorizer requires `syncBaseURL != nil`, but the + // SNTSyncdQueue caller invokes clearSyncState precisely when SyncBaseURL + // went to nil. Gating cleanup on the authorizer would make this caller a + // no-op. Lock the bypass in with a configurator whose authorizer always + // denies — clearSyncState must still drop in-memory state and the plist. + NSString* plistPath = [NSString stringWithFormat:@"%@/clear-authdenied.plist", self.testDir]; + + // Force a state file to exist on disk using a permissive configurator. + SNTConfigurator* writer = [self configuratorWithEmptySyncStateAtPath:plistPath]; + [writer setSyncServerClientMode:SNTClientModeLockdown]; + XCTAssertTrue([self.fileMgr fileExistsAtPath:plistPath]); + + // Now construct a new configurator over the same path with a denying + // authorizer and verify clearSyncState still cleans up. + SNTConfigurator* cfg = [[SNTConfigurator alloc] initWithSyncStateFile:plistPath + stateFile:@"/does/not/need/to/exist" + oldStateFile:@"/does/not/need/to/exist" + syncStateAccessAuthorizer:^BOOL { + return NO; + } + stateAccessAuthorizer:^BOOL { + return NO; + }]; + + [cfg clearSyncState]; + + XCTAssertEqual(cfg.syncState.count, (NSUInteger)0, + @"clearSyncState must reset in-memory state even when authorizer denies"); + XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath], + @"clearSyncState must remove the plist even when authorizer denies"); +} + @end diff --git a/Source/santad/SNTDaemonControlController.mm b/Source/santad/SNTDaemonControlController.mm index 2bde6e3be..a10ea4eee 100644 --- a/Source/santad/SNTDaemonControlController.mm +++ b/Source/santad/SNTDaemonControlController.mm @@ -482,138 +482,167 @@ - (void)disableUnknownEventUpload:(void (^)(BOOL))reply { - (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply { SNTConfigurator* configurator = [SNTConfigurator configurator]; - [result clientMode:^(SNTClientMode m) { - [configurator setSyncServerClientMode:m]; - }]; - - [result syncType:^(SNTSyncType val) { - [configurator setSyncTypeRequired:val]; - }]; - - [result allowlistRegex:^(NSString* val) { - [configurator - setSyncServerAllowedPathRegex:[NSRegularExpression regularExpressionWithPattern:val - options:0 - error:NULL]]; - }]; - - [result blocklistRegex:^(NSString* val) { - [configurator - setSyncServerBlockedPathRegex:[NSRegularExpression regularExpressionWithPattern:val - options:0 - error:NULL]]; - }]; - - [result removableMediaAction:^(NSString* val) { - [configurator setSyncServerRemovableMediaAction:val]; - }]; - - [result removableMediaRemountFlags:^(NSArray* val) { - [configurator setSyncServerRemovableMediaRemountFlags:val]; - }]; - - [result encryptedRemovableMediaAction:^(NSString* val) { - [configurator setSyncServerEncryptedRemovableMediaAction:val]; - }]; - - [result encryptedRemovableMediaRemountFlags:^(NSArray* val) { - [configurator setSyncServerEncryptedRemovableMediaRemountFlags:val]; - }]; - - [result blockNetworkMount:^(BOOL val) { - [configurator setSyncServerBlockNetworkMount:val]; - }]; - - [result bannedNetworkMountBlockMessage:^(NSString* val) { - [configurator setSyncServerBannedNetworkMountBlockMessage:val]; - }]; - - [result allowedNetworkMountHosts:^(NSArray* val) { - [configurator setSyncServerAllowedNetworkMountHosts:val]; - }]; - - [result enableBundles:^(BOOL val) { - [configurator setEnableBundles:val]; - }]; - - [result enableTransitiveRules:^(BOOL val) { - [configurator setEnableTransitiveRules:val]; - }]; - - [result enableAllEventUpload:^(BOOL val) { - [configurator setEnableAllEventUpload:val]; - }]; - - [result disableUnknownEventUpload:^(BOOL val) { - [configurator setDisableUnknownEventUpload:val]; - }]; - - [result overrideFileAccessAction:^(NSString* val) { - [configurator setSyncServerOverrideFileAccessAction:val]; - }]; - - [result exportConfiguration:^(SNTExportConfiguration* val) { - LOGD(@"Received export configuration: %@", val); - [configurator setSyncServerExportConfig:val]; - }]; - - [result fullSyncLastSuccess:^(NSDate* val) { - [configurator setFullSyncLastSuccess:val]; - }]; - - [result ruleSyncLastSuccess:^(NSDate* val) { - [configurator setFullSyncLastSuccess:val]; + // Snapshot CEL state to detect changes across the commit, including the + // "rules disappeared from the server's config on a clean sync" case where + // the bundle has no celFallbackRules slot but the wipe removed pre-existing + // rules. The post-batch read catches this uniformly with the rules-changed + // and rules-newly-added cases. + NSData* oldCELData = [SNTCELFallbackRule serializeArray:[configurator celFallbackRules]]; + + BOOL committed = [configurator performSyncStateBatch:^{ + [result clearSyncStateBeforeApply:^(BOOL clear) { + if (clear) { + [configurator clearSyncState]; + } + }]; + + // Persist the mode transition inside the batch so it commits atomically + // with the rest of sync-derived state. TMM's enforcement side effects + // (Revoke + GUI notify) run after the batch via NewModeTransitionReceived + // so that `Available(nil)` reads consistent post-commit state. + [result modeTransition:^(SNTModeTransition* val) { + [configurator setSyncServerModeTransition:val]; + }]; + + [result clientMode:^(SNTClientMode m) { + [configurator setSyncServerClientMode:m]; + }]; + + [result syncType:^(SNTSyncType val) { + [configurator setSyncTypeRequired:val]; + }]; + + [result allowlistRegex:^(NSString* val) { + [configurator + setSyncServerAllowedPathRegex:[NSRegularExpression regularExpressionWithPattern:val + options:0 + error:NULL]]; + }]; + + [result blocklistRegex:^(NSString* val) { + [configurator + setSyncServerBlockedPathRegex:[NSRegularExpression regularExpressionWithPattern:val + options:0 + error:NULL]]; + }]; + + [result removableMediaAction:^(NSString* val) { + [configurator setSyncServerRemovableMediaAction:val]; + }]; + + [result removableMediaRemountFlags:^(NSArray* val) { + [configurator setSyncServerRemovableMediaRemountFlags:val]; + }]; + + [result encryptedRemovableMediaAction:^(NSString* val) { + [configurator setSyncServerEncryptedRemovableMediaAction:val]; + }]; + + [result encryptedRemovableMediaRemountFlags:^(NSArray* val) { + [configurator setSyncServerEncryptedRemovableMediaRemountFlags:val]; + }]; + + [result blockNetworkMount:^(BOOL val) { + [configurator setSyncServerBlockNetworkMount:val]; + }]; + + [result bannedNetworkMountBlockMessage:^(NSString* val) { + [configurator setSyncServerBannedNetworkMountBlockMessage:val]; + }]; + + [result allowedNetworkMountHosts:^(NSArray* val) { + [configurator setSyncServerAllowedNetworkMountHosts:val]; + }]; + + [result enableBundles:^(BOOL val) { + [configurator setEnableBundles:val]; + }]; + + [result enableTransitiveRules:^(BOOL val) { + [configurator setEnableTransitiveRules:val]; + }]; + + [result enableAllEventUpload:^(BOOL val) { + [configurator setEnableAllEventUpload:val]; + }]; + + [result disableUnknownEventUpload:^(BOOL val) { + [configurator setDisableUnknownEventUpload:val]; + }]; + + [result overrideFileAccessAction:^(NSString* val) { + [configurator setSyncServerOverrideFileAccessAction:val]; + }]; + + [result exportConfiguration:^(SNTExportConfiguration* val) { + LOGD(@"Received export configuration: %@", val); + [configurator setSyncServerExportConfig:val]; + }]; + + [result fullSyncLastSuccess:^(NSDate* val) { + [configurator setFullSyncLastSuccess:val]; + }]; + + [result ruleSyncLastSuccess:^(NSDate* val) { + [configurator setRuleSyncLastSuccess:val]; + }]; + + [result networkExtensionSettings:^(SNTSyncNetworkExtensionSettings* val) { + [configurator setSyncServerSyncNetworkExtensionSettings:val]; + }]; + + [result pushTokenChain:^(NSArray* val) { + [configurator setSyncServerPushTokenChain:val]; + }]; + + [result telemetryFilterExpressions:^(NSArray* val) { + [configurator setSyncServerTelemetryFilterExpressions:val]; + }]; + + [result celFallbackRules:^(NSArray* val) { + [configurator setSyncServerCELFallbackRules:val]; + }]; + + [result eventDetailURL:^(NSString* val) { + [configurator setSyncServerEventDetailURL:val]; + }]; + + [result eventDetailText:^(NSString* val) { + [configurator setSyncServerEventDetailText:val]; + }]; + + [result fileAccessEventDetailURL:^(NSString* val) { + [configurator setSyncServerFileAccessEventDetailURL:val]; + }]; + + [result fileAccessEventDetailText:^(NSString* val) { + [configurator setSyncServerFileAccessEventDetailText:val]; + }]; + + [result fullSyncInterval:^(NSUInteger val) { + [configurator setSyncServerFullSyncInterval:val]; + }]; + + [result pushNotificationsFullSyncInterval:^(NSUInteger val) { + [configurator setSyncServerPushNotificationsFullSyncInterval:val]; + }]; }]; + // Mode-transition enforcement and GUI notification run after the batch so + // Available(nil) reads from the just-committed state. Bundle accessors are + // pure synchronous reads (see SNTConfigBundle.mm), so re-invoking the same + // callback shape is safe and short-circuits when the bundle carries no + // modeTransition. [result modeTransition:^(SNTModeTransition* val) { - // The _temporaryMonitorMode object is responsible for updating configurator as appropriate _temporaryMonitorMode->NewModeTransitionReceived(val); }]; - [result networkExtensionSettings:^(SNTSyncNetworkExtensionSettings* val) { - [configurator setSyncServerSyncNetworkExtensionSettings:val]; - }]; - - [result pushTokenChain:^(NSArray* val) { - [configurator setSyncServerPushTokenChain:val]; - }]; - - [result telemetryFilterExpressions:^(NSArray* val) { - [configurator setSyncServerTelemetryFilterExpressions:val]; - }]; - - [result celFallbackRules:^(NSArray* val) { - NSData* oldData = [SNTCELFallbackRule serializeArray:[configurator celFallbackRules]]; - NSData* newData = [SNTCELFallbackRule serializeArray:val]; - [configurator setSyncServerCELFallbackRules:val]; - if (oldData != newData && ![oldData isEqualToData:newData] && self.flushCacheBlock) { + if (committed) { + NSData* newCELData = [SNTCELFallbackRule serializeArray:[configurator celFallbackRules]]; + if (![oldCELData isEqualToData:newCELData] && self.flushCacheBlock) { self.flushCacheBlock(FlushCacheMode::kAllCaches, FlushCacheReason::kCELFallbackRulesChanged); } - }]; - - [result eventDetailURL:^(NSString* val) { - [configurator setSyncServerEventDetailURL:val]; - }]; - - [result eventDetailText:^(NSString* val) { - [configurator setSyncServerEventDetailText:val]; - }]; - - [result fileAccessEventDetailURL:^(NSString* val) { - [configurator setSyncServerFileAccessEventDetailURL:val]; - }]; - - [result fileAccessEventDetailText:^(NSString* val) { - [configurator setSyncServerFileAccessEventDetailText:val]; - }]; - - [result fullSyncInterval:^(NSUInteger val) { - [configurator setSyncServerFullSyncInterval:val]; - }]; - - [result pushNotificationsFullSyncInterval:^(NSUInteger val) { - [configurator setSyncServerPushNotificationsFullSyncInterval:val]; - }]; + } reply(); } diff --git a/Source/santad/TemporaryMonitorMode.mm b/Source/santad/TemporaryMonitorMode.mm index 35abe1dc5..9092af0e1 100644 --- a/Source/santad/TemporaryMonitorMode.mm +++ b/Source/santad/TemporaryMonitorMode.mm @@ -149,15 +149,17 @@ } void TemporaryMonitorMode::NewModeTransitionReceived(SNTModeTransition* mode_transition) { + // Persistence of the received mode_transition is the caller's responsibility + // (the daemon controller writes it inside a sync-state batch so that all + // sync-derived state lands in a single commit). This method handles + // enforcement side effects for Revoke and the GUI availability notification, + // both of which must run regardless of how persistence was sequenced. if (mode_transition.type == SNTModeTransitionTypeRevoke) { if (Revoke(SNTTemporaryMonitorModeLeaveReasonRevoked)) { LOGI(@"Temporary Monitor Mode session revoked due to policy change."); } - } else { - [configurator_ setSyncServerModeTransition:mode_transition]; } - // Notify the GUI about policy availability [[notification_queue_.notifierConnection remoteObjectProxy] temporaryMonitorModePolicyAvailable:Available(nil)]; } @@ -287,7 +289,14 @@ } bool TemporaryMonitorMode::RevokeLocked(SNTTemporaryMonitorModeLeaveReason reason) { - [configurator_ setSyncServerModeTransition:[[SNTModeTransition alloc] initRevocation]]; + // Skip the persistence write if the current effective transition is already + // a revoke. On the sync-update path the daemon controller writes the revoke + // inside its batch, so this would otherwise produce a second `syncState` + // KVO fire. On all other Revoke entry points (sync URL change, etc.) the + // current transition will not be a revoke and this writes as before. + if ([configurator_ modeTransition].type != SNTModeTransitionTypeRevoke) { + [configurator_ setSyncServerModeTransition:[[SNTModeTransition alloc] initRevocation]]; + } if (EndLocked()) { handle_audit_event_block_([[SNTStoredTemporaryMonitorModeLeaveAuditEvent alloc] initWithUUID:[current_uuid_ UUIDString] diff --git a/Source/santasyncservice/SNTSyncConfigBundle.mm b/Source/santasyncservice/SNTSyncConfigBundle.mm index 9781b0e88..22a8e2a3e 100644 --- a/Source/santasyncservice/SNTSyncConfigBundle.mm +++ b/Source/santasyncservice/SNTSyncConfigBundle.mm @@ -52,6 +52,7 @@ @interface SNTConfigBundle (ConfigBundleCreator) @property NSArray* celFallbackRules; @property NSNumber* fullSyncInterval; @property NSNumber* pushNotificationsFullSyncInterval; +@property NSNumber* clearSyncStateBeforeApply; @end SNTConfigBundle* PreflightConfigBundle(SNTSyncState* syncState) { @@ -97,6 +98,13 @@ @interface SNTConfigBundle (ConfigBundleCreator) bundle.fullSyncLastSuccess = [NSDate now]; + if (syncState.syncType != SNTSyncTypeNormal) { + bundle.clearSyncStateBeforeApply = @(YES); + } + if (syncState.pushIssuerJWT.length && syncState.pushJWT.length) { + bundle.pushTokenChain = @[ syncState.pushIssuerJWT, syncState.pushJWT ]; + } + return bundle; } diff --git a/Source/santasyncservice/SNTSyncConfigBundleTest.mm b/Source/santasyncservice/SNTSyncConfigBundleTest.mm index caca79cfe..06fdee91a 100644 --- a/Source/santasyncservice/SNTSyncConfigBundleTest.mm +++ b/Source/santasyncservice/SNTSyncConfigBundleTest.mm @@ -55,6 +55,7 @@ @interface SNTConfigBundle (ConfigBundleCreator) @property NSArray* celFallbackRules; @property NSNumber* fullSyncInterval; @property NSNumber* pushNotificationsFullSyncInterval; +@property NSNumber* clearSyncStateBeforeApply; @end @interface SNTSyncConfigBundleTest : XCTestCase @@ -258,4 +259,89 @@ - (void)testSyncTypeConfigBundle { XCTAssertNil(bundle.celFallbackRules); } +- (void)testPostflightConfigBundleSetsClearSyncStateBeforeApplyByInflightSyncType { + struct { + SNTSyncType inflight; + BOOL expectFlagFires; + } cases[] = { + {SNTSyncTypeNormal, NO}, + {SNTSyncTypeClean, YES}, + {SNTSyncTypeCleanAll, YES}, + }; + + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) { + SNTSyncState* state = [[SNTSyncState alloc] init]; + state.syncType = cases[i].inflight; + SNTConfigBundle* bundle = PostflightConfigBundle(state); + + __block BOOL fired = NO; + __block BOOL value = NO; + [bundle clearSyncStateBeforeApply:^(BOOL v) { + fired = YES; + value = v; + }]; + + if (cases[i].expectFlagFires) { + XCTAssertTrue(fired, + @"PostflightConfigBundle must set clearSyncStateBeforeApply for syncType=%lu", + (unsigned long)cases[i].inflight); + XCTAssertEqual(value, YES); + } else { + XCTAssertFalse( + fired, @"PostflightConfigBundle must NOT set clearSyncStateBeforeApply for Normal sync"); + } + } +} + +- (void)testPostflightConfigBundleForwardsPushTokenChainWhenBothJWTsPresent { + SNTSyncState* state = [[SNTSyncState alloc] init]; + state.pushIssuerJWT = @"issuerToken"; + state.pushJWT = @"userToken"; + + SNTConfigBundle* bundle = PostflightConfigBundle(state); + XCTAssertEqualObjects(bundle.pushTokenChain, (@[ @"issuerToken", @"userToken" ])); +} + +- (void)testPostflightConfigBundleOmitsPushTokenChainWhenEitherJWTMissing { + // Both missing + { + SNTSyncState* state = [[SNTSyncState alloc] init]; + SNTConfigBundle* bundle = PostflightConfigBundle(state); + XCTAssertNil(bundle.pushTokenChain); + } + // Only issuer present + { + SNTSyncState* state = [[SNTSyncState alloc] init]; + state.pushIssuerJWT = @"issuerToken"; + SNTConfigBundle* bundle = PostflightConfigBundle(state); + XCTAssertNil(bundle.pushTokenChain); + } + // Only user present + { + SNTSyncState* state = [[SNTSyncState alloc] init]; + state.pushJWT = @"userToken"; + SNTConfigBundle* bundle = PostflightConfigBundle(state); + XCTAssertNil(bundle.pushTokenChain); + } +} + +- (void)testNonPostflightFactoriesNeverSetClearSyncStateBeforeApply { + SNTSyncState* state = [[SNTSyncState alloc] init]; + + SNTConfigBundle* bundles[] = { + PreflightConfigBundle(state), + RuleSyncConfigBundle(), + SyncTypeConfigBundle(SNTSyncTypeClean), + }; + NSString* names[] = {@"PreflightConfigBundle", @"RuleSyncConfigBundle", @"SyncTypeConfigBundle"}; + + for (size_t i = 0; i < sizeof(bundles) / sizeof(bundles[0]); ++i) { + __block BOOL fired = NO; + [bundles[i] clearSyncStateBeforeApply:^(BOOL v) { + fired = YES; + }]; + XCTAssertFalse(fired, @"%@ must not set clearSyncStateBeforeApply", names[i]); + } +} + @end diff --git a/Source/santasyncservice/SNTSyncTest.mm b/Source/santasyncservice/SNTSyncTest.mm index d14a0273a..0a0c6af99 100644 --- a/Source/santasyncservice/SNTSyncTest.mm +++ b/Source/santasyncservice/SNTSyncTest.mm @@ -1208,6 +1208,69 @@ - (void)testPostflightBasicResponse { OCMVerify([self.daemonConnRop updateSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); } +- (SNTConfigBundle*)runPostflightAndCaptureBundleWithInflightSyncType:(SNTSyncType)inflight { + [self setupDefaultDaemonConnResponses]; + self.syncState.syncType = inflight; + + __block SNTConfigBundle* captured = nil; + OCMStub([self.daemonConnRop updateSyncSettings:[OCMArg any] reply:[OCMArg any]]) + .andDo(^(NSInvocation* inv) { + SNTConfigBundle* __unsafe_unretained bundle = nil; + [inv getArgument:&bundle atIndex:2]; + captured = bundle; + void (^__unsafe_unretained replyBlock)(void) = nil; + [inv getArgument:&replyBlock atIndex:3]; + if (replyBlock) replyBlock(); + }); + + SNTSyncPostflight* sut = [[SNTSyncPostflight alloc] initWithState:self.syncState]; + [self stubRequestBody:nil response:nil error:nil validateBlock:nil]; + XCTAssertTrue([sut sync]); + XCTAssertNotNil(captured, @"postflight must ship a bundle via updateSyncSettings:"); + return captured; +} + +- (void)testPostflightOnCleanSyncShipsBundleWithClearSyncStateBeforeApply { + SNTConfigBundle* bundle = + [self runPostflightAndCaptureBundleWithInflightSyncType:SNTSyncTypeClean]; + + __block BOOL fired = NO; + __block BOOL value = NO; + [bundle clearSyncStateBeforeApply:^(BOOL v) { + fired = YES; + value = v; + }]; + XCTAssertTrue(fired, @"Postflight bundle must carry clearSyncStateBeforeApply on a Clean sync"); + XCTAssertEqual(value, YES); +} + +- (void)testPostflightOnCleanAllSyncShipsBundleWithClearSyncStateBeforeApply { + SNTConfigBundle* bundle = + [self runPostflightAndCaptureBundleWithInflightSyncType:SNTSyncTypeCleanAll]; + + __block BOOL fired = NO; + __block BOOL value = NO; + [bundle clearSyncStateBeforeApply:^(BOOL v) { + fired = YES; + value = v; + }]; + XCTAssertTrue(fired, + @"Postflight bundle must carry clearSyncStateBeforeApply on a CleanAll sync"); + XCTAssertEqual(value, YES); +} + +- (void)testPostflightOnNormalSyncShipsBundleWithoutClearSyncStateBeforeApply { + SNTConfigBundle* bundle = + [self runPostflightAndCaptureBundleWithInflightSyncType:SNTSyncTypeNormal]; + + __block BOOL fired = NO; + [bundle clearSyncStateBeforeApply:^(BOOL v) { + fired = YES; + }]; + XCTAssertFalse(fired, + @"Postflight bundle must NOT carry clearSyncStateBeforeApply on a Normal sync"); +} + #pragma mark - Dynamic NATS Push Client Lifecycle Tests - (void)testPreflightCreatesNATSWhenSyncV2 {