From ed4bac4370d2384d8acca4c96c40a59311aa015c Mon Sep 17 00:00:00 2001 From: Sharvil Shah Date: Tue, 5 May 2026 21:30:56 +0530 Subject: [PATCH 1/6] fixes snt-357 --- Source/common/BUILD | 2 + Source/common/SNTConfigurator.h | 14 ++ Source/common/SNTConfigurator.mm | 138 +++++++++++++++- Source/common/SNTConfiguratorTest.mm | 156 ++++++++++++++++++ Source/common/SNTXPCControlInterface.h | 8 + Source/common/SNTXPCSyncServiceInterface.h | 1 + Source/gui/SNTAboutWindowView.swift | 6 +- Source/gui/SNTStatusItemManager.mm | 1 + Source/santactl/Commands/SNTCommandSync.mm | 36 ++-- Source/santad/SNTDaemonControlController.mm | 5 + .../SNTSyncConfigBundleTest.mm | 35 ++++ Source/santasyncservice/SNTSyncManager.h | 12 +- Source/santasyncservice/SNTSyncManager.mm | 13 +- Source/santasyncservice/SNTSyncPostflight.mm | 15 +- Source/santasyncservice/SNTSyncService.mm | 2 + Source/santasyncservice/SNTSyncState.h | 7 + Source/santasyncservice/SNTSyncTest.mm | 52 ++++++ 17 files changed, 478 insertions(+), 25 deletions(-) diff --git a/Source/common/BUILD b/Source/common/BUILD index b10b9fc2a..b6666cfdb 100644 --- a/Source/common/BUILD +++ b/Source/common/BUILD @@ -514,6 +514,7 @@ objc_library( ":Pinning", ":SNTCELFallbackRule", ":SNTCommonEnums", + ":SNTConfigBundle", ":SNTExportConfiguration", ":SNTLiteDetector", ":SNTLogging", @@ -1094,6 +1095,7 @@ santa_unit_test( srcs = ["SNTConfiguratorTest.mm"], deps = [ ":SNTCommonEnums", + ":SNTConfigBundle", ":SNTConfigurator", "@OCMock", ], diff --git a/Source/common/SNTConfigurator.h b/Source/common/SNTConfigurator.h index 77f2c69b2..698c3c6b0 100644 --- a/Source/common/SNTConfigurator.h +++ b/Source/common/SNTConfigurator.h @@ -16,6 +16,7 @@ #import #import "Source/common/SNTCommonEnums.h" +#import "Source/common/SNTConfigBundle.h" @class SNTCELFallbackRule; @class SNTExportConfiguration; @@ -1152,6 +1153,19 @@ extern NSString* _Nonnull const kEnableMenuItemUserOverride; /// - (void)clearSyncState; +/// +/// Atomically replace the persisted sync state with a new dictionary +/// containing only the existing `PushTokenChain` (if present) plus any +/// non-nil values from `bundle`. Performs a single `saveSyncStateToDisk` +/// via `writeToFile:atomically:YES`. +/// +/// Used by the daemon's `replaceSyncSettings:reply:` XPC handler when a +/// clean sync's postflight is committing settings under the SNT-357 +/// semantics. Workshop-managed keys not present in `bundle` are +/// cleared from disk by this call. +/// +- (void)replaceSyncStateWithBundle:(nonnull SNTConfigBundle*)bundle; + /// /// Validate the configuration profile. /// diff --git a/Source/common/SNTConfigurator.mm b/Source/common/SNTConfigurator.mm index 11fe8bb06..35adf64d4 100644 --- a/Source/common/SNTConfigurator.mm +++ b/Source/common/SNTConfigurator.mm @@ -1959,11 +1959,17 @@ - (NSDictionary*)extraMetricLabels { /// immediately after the call completes. /// - (void)updateSyncStateForKey:(NSString*)key value:(id)value { + [self updateSyncStateForKey:key value:value writeToDisk:YES]; +} + +- (void)updateSyncStateForKey:(NSString*)key value:(id)value writeToDisk:(BOOL)writeToDisk { void (^block)(void) = ^{ NSMutableDictionary* syncState = self.syncState.mutableCopy; syncState[key] = value; self.syncState = syncState; - [self saveSyncStateToDisk]; + if (writeToDisk) { + [self saveSyncStateToDisk]; + } }; // Avoid deadlocks by directly calling the block when already running on the // main thread. @@ -2061,9 +2067,133 @@ - (void)saveSyncStateToDisk { - (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. + [[NSFileManager defaultManager] removeItemAtPath:self.syncStateFilePath error:NULL]; +} + +- (void)replaceSyncStateWithBundle:(SNTConfigBundle*)bundle { + void (^block)(void) = ^{ + NSMutableDictionary* newSyncState = [NSMutableDictionary dictionary]; + + NSArray* preservedChain = self.syncState[kPushTokenChainKey]; + if ([preservedChain isKindOfClass:[NSArray class]] && preservedChain.count > 0) { + newSyncState[kPushTokenChainKey] = preservedChain; + } + + self.syncState = newSyncState; + + [self applySyncBundleSettersDeferringDiskWrites:bundle]; + + [self saveSyncStateToDisk]; + }; + + if ([NSThread isMainThread]) { + block(); + } else { + dispatch_sync(dispatch_get_main_queue(), block); + } +} + +- (void)applySyncBundleSettersDeferringDiskWrites:(SNTConfigBundle*)bundle { + [bundle clientMode:^(SNTClientMode m) { + [self updateSyncStateForKey:kClientModeKey value:@(m) writeToDisk:NO]; + }]; + [bundle syncType:^(SNTSyncType val) { + [self updateSyncStateForKey:kSyncTypeRequired value:@(val) writeToDisk:NO]; + }]; + [bundle allowlistRegex:^(NSString* val) { + [self updateSyncStateForKey:kAllowedPathRegexKey + value:[NSRegularExpression regularExpressionWithPattern:val + options:0 + error:NULL] + writeToDisk:NO]; + }]; + [bundle blocklistRegex:^(NSString* val) { + [self updateSyncStateForKey:kBlockedPathRegexKey + value:[NSRegularExpression regularExpressionWithPattern:val + options:0 + error:NULL] + writeToDisk:NO]; + }]; + [bundle removableMediaAction:^(NSString* val) { + [self updateSyncStateForKey:kRemovableMediaActionKey value:val writeToDisk:NO]; + }]; + [bundle removableMediaRemountFlags:^(NSArray* val) { + [self updateSyncStateForKey:kRemovableMediaRemountFlagsKey value:val writeToDisk:NO]; + }]; + [bundle encryptedRemovableMediaAction:^(NSString* val) { + [self updateSyncStateForKey:kEncryptedRemovableMediaActionKey value:val writeToDisk:NO]; + }]; + [bundle encryptedRemovableMediaRemountFlags:^(NSArray* val) { + [self updateSyncStateForKey:kEncryptedRemovableMediaRemountFlagsKey value:val writeToDisk:NO]; + }]; + [bundle blockNetworkMount:^(BOOL val) { + [self updateSyncStateForKey:kBlockNetworkMountKey value:@(val) writeToDisk:NO]; + }]; + [bundle bannedNetworkMountBlockMessage:^(NSString* val) { + [self updateSyncStateForKey:kBannedNetworkMountBlockMessage value:val writeToDisk:NO]; + }]; + [bundle allowedNetworkMountHosts:^(NSArray* val) { + [self updateSyncStateForKey:kAllowedNetworkMountHosts value:val writeToDisk:NO]; + }]; + [bundle enableBundles:^(BOOL val) { + [self updateSyncStateForKey:kEnableBundlesKey value:@(val) writeToDisk:NO]; + }]; + [bundle enableTransitiveRules:^(BOOL val) { + [self updateSyncStateForKey:kEnableTransitiveRulesKey value:@(val) writeToDisk:NO]; + }]; + [bundle enableAllEventUpload:^(BOOL val) { + [self updateSyncStateForKey:kEnableAllEventUploadKey value:@(val) writeToDisk:NO]; + }]; + [bundle disableUnknownEventUpload:^(BOOL val) { + [self updateSyncStateForKey:kDisableUnknownEventUploadKey value:@(val) writeToDisk:NO]; + }]; + [bundle overrideFileAccessAction:^(NSString* val) { + [self updateSyncStateForKey:kOverrideFileAccessActionKey value:val writeToDisk:NO]; + }]; + [bundle exportConfiguration:^(SNTExportConfiguration* val) { + [self updateSyncStateForKey:kExportConfigurationKey value:[val serialize] writeToDisk:NO]; + }]; + [bundle fullSyncLastSuccess:^(NSDate* val) { + [self updateSyncStateForKey:kFullSyncLastSuccess value:val writeToDisk:NO]; + }]; + [bundle ruleSyncLastSuccess:^(NSDate* val) { + [self updateSyncStateForKey:kRuleSyncLastSuccess value:val writeToDisk:NO]; + }]; + [bundle modeTransition:^(SNTModeTransition* val) { + [self updateSyncStateForKey:kModeTransitionKey value:[val serialize] writeToDisk:NO]; + }]; + [bundle networkExtensionSettings:^(SNTSyncNetworkExtensionSettings* val) { + [self updateSyncStateForKey:kNetworkExtensionSettingsKey value:[val serialize] writeToDisk:NO]; + }]; + [bundle pushTokenChain:^(NSArray* val) { + [self updateSyncStateForKey:kPushTokenChainKey value:val writeToDisk:NO]; + }]; + [bundle telemetryFilterExpressions:^(NSArray* val) { + [self updateSyncStateForKey:kTelemetryFilterExpressionsKey value:val writeToDisk:NO]; + }]; + [bundle celFallbackRules:^(NSArray* val) { + [self updateSyncStateForKey:kCELFallbackRulesKey + value:[SNTCELFallbackRule serializeArray:val] + writeToDisk:NO]; + }]; + [bundle eventDetailURL:^(NSString* val) { + [self updateSyncStateForKey:kEventDetailURLKey value:val writeToDisk:NO]; + }]; + [bundle eventDetailText:^(NSString* val) { + [self updateSyncStateForKey:kEventDetailTextKey value:val writeToDisk:NO]; + }]; + [bundle fileAccessEventDetailURL:^(NSString* val) { + [self updateSyncStateForKey:kFileAccessEventDetailURLKey value:val writeToDisk:NO]; + }]; + [bundle fileAccessEventDetailText:^(NSString* val) { + [self updateSyncStateForKey:kFileAccessEventDetailTextKey value:val writeToDisk:NO]; + }]; + [bundle fullSyncInterval:^(NSUInteger val) { + [self updateSyncStateForKey:kFullSyncInterval value:val ? @(val) : nil writeToDisk:NO]; + }]; + [bundle pushNotificationsFullSyncInterval:^(NSUInteger val) { + [self updateSyncStateForKey:kFCMFullSyncInterval value:val ? @(val) : nil writeToDisk:NO]; + }]; } - (NSArray*)entitlementsPrefixFilter { diff --git a/Source/common/SNTConfiguratorTest.mm b/Source/common/SNTConfiguratorTest.mm index a95d63778..277032ca0 100644 --- a/Source/common/SNTConfiguratorTest.mm +++ b/Source/common/SNTConfiguratorTest.mm @@ -17,6 +17,7 @@ #import #import "Source/common/SNTCommonEnums.h" +#import "Source/common/SNTConfigBundle.h" #import "Source/common/SNTConfigurator.h" typedef BOOL (^StateFileAccessAuthorizer)(void); @@ -32,6 +33,15 @@ - (instancetype)initWithSyncStateFile:(NSString*)syncStateFilePath @property NSMutableDictionary* syncState; @end +// Expose SNTConfigBundle's writable properties for test setup. These properties +// are declared as a class extension in SNTConfigBundle.mm; redeclaring them as +// a category here gives the test typed setter access. +@interface SNTConfigBundle (ConfigBundleCreator) +@property NSNumber* clientMode; +@property NSDate* fullSyncLastSuccess; +@property NSArray* pushTokenChain; +@end + @interface SNTConfiguratorTest : XCTestCase @property NSFileManager* fileMgr; @property NSString* testDir; @@ -285,4 +295,150 @@ - (void)testAllowDelegatedSignalsOverride { XCTAssertFalse(sut.allowDelegatedSignals); } +#pragma mark - replaceSyncStateWithBundle: tests + +- (SNTConfigurator*)configuratorWithSyncState:(NSDictionary*)initialState + plistPath:(NSString*)plistPath { + if (initialState) { + XCTAssertTrue([initialState writeToFile:plistPath atomically:YES]); + } + 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)testReplaceSyncStateWithBundleClearsWorkshopKeysPreservingPushTokenChain { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-replace-state.plist", self.testDir]; + NSArray* chain = @[ @"issuer-jwt-value", @"device-jwt-value" ]; + + SNTConfigurator* cfg = [self configuratorWithSyncState:@{ + @"ClientMode" : @(SNTClientModeLockdown), + @"AllowedPathRegex" : @"old-allow-pattern", + @"PushTokenChain" : chain, + } + plistPath:plistPath]; + + SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; + bundle.clientMode = @(SNTClientModeMonitor); + bundle.fullSyncLastSuccess = [NSDate date]; + + [cfg replaceSyncStateWithBundle:bundle]; + + NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; + XCTAssertEqualObjects(onDisk[@"PushTokenChain"], chain); + XCTAssertEqual([onDisk[@"ClientMode"] integerValue], SNTClientModeMonitor); + XCTAssertNotNil(onDisk[@"FullSyncLastSuccess"]); + XCTAssertNil(onDisk[@"AllowedPathRegex"]); + + XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); +} + +- (void)testReplaceSyncStateWithBundleNoExistingChainProducesNoChainOnDisk { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-replace-state.plist", self.testDir]; + + SNTConfigurator* cfg = [self configuratorWithSyncState:@{ + @"ClientMode" : @(SNTClientModeLockdown), + } + plistPath:plistPath]; + + SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; + bundle.clientMode = @(SNTClientModeMonitor); + + [cfg replaceSyncStateWithBundle:bundle]; + + NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; + XCTAssertNil(onDisk[@"PushTokenChain"]); + XCTAssertEqual([onDisk[@"ClientMode"] integerValue], SNTClientModeMonitor); + + XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); +} + +- (void)testReplaceSyncStateWithBundleEmptyBundleLeavesOnlyChain { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-replace-state.plist", self.testDir]; + NSArray* chain = @[ @"issuer", @"device" ]; + + SNTConfigurator* cfg = [self configuratorWithSyncState:@{ + @"ClientMode" : @(SNTClientModeLockdown), + @"PushTokenChain" : chain, + } + plistPath:plistPath]; + + SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; + + [cfg replaceSyncStateWithBundle:bundle]; + + NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; + XCTAssertEqualObjects(onDisk[@"PushTokenChain"], chain); + XCTAssertEqual(onDisk.count, (NSUInteger)1); + + XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); +} + +- (void)testReplaceSyncStateWithBundleBundleChainOverridesPreserved { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-replace-state.plist", self.testDir]; + NSArray* staleChain = @[ @"old-issuer", @"old-device" ]; + NSArray* freshChain = @[ @"new-issuer", @"new-device" ]; + + SNTConfigurator* cfg = [self configuratorWithSyncState:@{ + @"PushTokenChain" : staleChain, + } + plistPath:plistPath]; + + SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; + bundle.pushTokenChain = freshChain; + + [cfg replaceSyncStateWithBundle:bundle]; + + NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; + XCTAssertEqualObjects(onDisk[@"PushTokenChain"], freshChain); + + XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); +} + +#pragma mark - clearSyncState tests (Bug 2) + +- (void)testClearSyncStateRemovesDiskFile { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-clear-state.plist", self.testDir]; + NSDictionary* contents = @{@"ClientMode" : @(SNTClientModeLockdown)}; + XCTAssertTrue([contents writeToFile:plistPath atomically:YES]); + XCTAssertTrue([self.fileMgr fileExistsAtPath:plistPath]); + + SNTConfigurator* cfg = [self configuratorWithSyncState:nil plistPath:plistPath]; + cfg.syncState = contents.mutableCopy; + + [cfg clearSyncState]; + + XCTAssertEqual(cfg.syncState.count, (NSUInteger)0); + XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]); +} + +- (void)testClearSyncStateNoFileDoesNotError { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-clear-state.plist", self.testDir]; + XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]); + + SNTConfigurator* cfg = [self configuratorWithSyncState:nil plistPath:plistPath]; + + XCTAssertNoThrow([cfg clearSyncState]); + XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]); +} + +- (void)testClearSyncStateIdempotent { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-clear-state.plist", self.testDir]; + NSDictionary* contents = @{@"ClientMode" : @(SNTClientModeLockdown)}; + XCTAssertTrue([contents writeToFile:plistPath atomically:YES]); + + SNTConfigurator* cfg = [self configuratorWithSyncState:nil plistPath:plistPath]; + cfg.syncState = contents.mutableCopy; + + [cfg clearSyncState]; + XCTAssertNoThrow([cfg clearSyncState]); + XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]); +} + @end diff --git a/Source/common/SNTXPCControlInterface.h b/Source/common/SNTXPCControlInterface.h index 8bd0c41ad..2e2b3febb 100644 --- a/Source/common/SNTXPCControlInterface.h +++ b/Source/common/SNTXPCControlInterface.h @@ -57,6 +57,14 @@ typedef NS_ENUM(NSInteger, SNTRuleAddSource) { /// - (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply; +/// +/// Atomically replace the persisted sync state with the contents of `bundle`, +/// preserving only `PushTokenChain` from the previous state. Used by the +/// sync service's postflight stage when a clean sync (Clean or CleanAll) +/// resolves and the user did not pass `--keep-old-settings`. +/// +- (void)replaceSyncSettings:(SNTConfigBundle*)bundle reply:(void (^)(void))reply; + /// /// Syncd Ops /// diff --git a/Source/common/SNTXPCSyncServiceInterface.h b/Source/common/SNTXPCSyncServiceInterface.h index db8a0747b..562d9f36f 100644 --- a/Source/common/SNTXPCSyncServiceInterface.h +++ b/Source/common/SNTXPCSyncServiceInterface.h @@ -49,6 +49,7 @@ // - (void)syncWithLogListener:(NSXPCListenerEndpoint*)logListener syncType:(SNTSyncType)syncType + keepOldSettings:(BOOL)keepOldSettings reply:(void (^)(SNTSyncStatusType))reply; // Publish metrics to the sync server. The metrics dictionary is the output of SNTMetricSet export. diff --git a/Source/gui/SNTAboutWindowView.swift b/Source/gui/SNTAboutWindowView.swift index f7d5766a1..17d8cf27e 100644 --- a/Source/gui/SNTAboutWindowView.swift +++ b/Source/gui/SNTAboutWindowView.swift @@ -206,7 +206,11 @@ struct SyncButtonView: View { lr?.resume() let proxy = ss?.remoteObjectProxy as? SNTSyncServiceXPC - proxy?.sync(withLogListener: logListener.endpoint, syncType: clean ? .clean : .normal) { status in + proxy?.sync( + withLogListener: logListener.endpoint, + syncType: clean ? .clean : .normal, + keepOldSettings: false + ) { status in lr = nil DispatchQueue.main.sync { diff --git a/Source/gui/SNTStatusItemManager.mm b/Source/gui/SNTStatusItemManager.mm index b08167b65..920393ca1 100644 --- a/Source/gui/SNTStatusItemManager.mm +++ b/Source/gui/SNTStatusItemManager.mm @@ -362,6 +362,7 @@ - (void)syncMenuItemClicked:(id)sender { [[ss synchronousRemoteObjectProxy] syncWithLogListener:nil syncType:SNTSyncTypeNormal + keepOldSettings:NO reply:^(SNTSyncStatusType status) { dispatch_async(dispatch_get_main_queue(), ^{ self.syncMenuItem.target = self; diff --git a/Source/santactl/Commands/SNTCommandSync.mm b/Source/santactl/Commands/SNTCommandSync.mm index 069555bcf..8cec9d4d2 100644 --- a/Source/santactl/Commands/SNTCommandSync.mm +++ b/Source/santactl/Commands/SNTCommandSync.mm @@ -51,9 +51,14 @@ + (NSString*)longHelpText { @"this is the command used for syncing.\n\n" @"Options:\n" @" --clean: Perform a clean sync, erasing all existing non-transitive rules and\n" - @" requesting a clean sync from the server.\n" + @" requesting a clean sync from the server. Also clears Workshop-managed\n" + @" sync settings; pass --keep-old-settings to retain them.\n" @" --clean-all: Perform a clean sync, erasing all existing rules and requesting a\n" - @" clean sync from the server.\n" + @" clean sync from the server. Also clears Workshop-managed sync\n" + @" settings; pass --keep-old-settings to retain them.\n" + @" --keep-old-settings: Only valid with --clean or --clean-all. Preserve existing\n" + @" Workshop-managed sync settings on disk during the clean\n" + @" sync, instead of clearing them.\n" @" --debug: Enable verbose output.\n"); } @@ -68,6 +73,23 @@ - (void)runWithArguments:(NSArray*)arguments { TEE_LOGE(@"Missing SyncBaseURL. Exiting."); exit(1); } + + NSArray* args = NSProcessInfo.processInfo.arguments; + SNTSyncType syncType = SNTSyncTypeNormal; + if ([args containsObject:@"--clean-all"]) { + syncType = SNTSyncTypeCleanAll; + } else if ([args containsObject:@"--clean"]) { + syncType = SNTSyncTypeClean; + } + + BOOL keepOldSettings = [args containsObject:@"--keep-old-settings"]; + if (keepOldSettings && syncType == SNTSyncTypeNormal) { + TEE_LOGE(@"--keep-old-settings requires --clean or --clean-all. Exiting."); + exit(1); + } + + self.enableDebugLogging = [args containsObject:@"--debug"]; + MOLXPCConnection* ss = [SNTXPCSyncServiceInterface configuredConnection]; ss.invalidationHandler = ^(void) { TEE_LOGE(@"Failed to connect to the sync service."); @@ -82,18 +104,10 @@ - (void)runWithArguments:(NSArray*)arguments { [NSXPCInterface interfaceWithProtocol:@protocol(SNTSyncServiceLogReceiverXPC)]; [lr resume]; - SNTSyncType syncType = SNTSyncTypeNormal; - if ([NSProcessInfo.processInfo.arguments containsObject:@"--clean-all"]) { - syncType = SNTSyncTypeCleanAll; - } else if ([NSProcessInfo.processInfo.arguments containsObject:@"--clean"]) { - syncType = SNTSyncTypeClean; - } - - self.enableDebugLogging = [NSProcessInfo.processInfo.arguments containsObject:@"--debug"]; - [[ss remoteObjectProxy] syncWithLogListener:logListener.endpoint syncType:syncType + keepOldSettings:keepOldSettings reply:^(SNTSyncStatusType status) { if (status == SNTSyncStatusTypeTooManySyncsInProgress) { [self didReceiveLog:@"Too many syncs in progress, try again later." diff --git a/Source/santad/SNTDaemonControlController.mm b/Source/santad/SNTDaemonControlController.mm index 99f28aea5..5aaf9b0f2 100644 --- a/Source/santad/SNTDaemonControlController.mm +++ b/Source/santad/SNTDaemonControlController.mm @@ -609,6 +609,11 @@ - (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply reply(); } +- (void)replaceSyncSettings:(SNTConfigBundle*)bundle reply:(void (^)(void))reply { + [[SNTConfigurator configurator] replaceSyncStateWithBundle:bundle]; + reply(); +} + - (void)retrieveStatsState:(void (^)(NSDate*, NSString*))reply { reply([[SNTConfigurator configurator] lastStatsSubmissionTimestamp], [[SNTConfigurator configurator] lastStatsSubmissionVersion]); diff --git a/Source/santasyncservice/SNTSyncConfigBundleTest.mm b/Source/santasyncservice/SNTSyncConfigBundleTest.mm index caca79cfe..79dd60508 100644 --- a/Source/santasyncservice/SNTSyncConfigBundleTest.mm +++ b/Source/santasyncservice/SNTSyncConfigBundleTest.mm @@ -258,4 +258,39 @@ - (void)testSyncTypeConfigBundle { XCTAssertNil(bundle.celFallbackRules); } +// PostflightConfigBundle invariants: these tests pin behavior the SNT-357 +// atomic-replace path depends on. If a future refactor breaks either invariant, +// `replaceSyncStateWithBundle:` would silently leave the on-disk plist in an +// inconsistent state after a clean sync. + +- (void)testPostflightConfigBundleAlwaysSetsFullSyncLastSuccess { + for (SNTSyncType type : + (SNTSyncType[]){SNTSyncTypeNormal, SNTSyncTypeClean, SNTSyncTypeCleanAll}) { + SNTSyncState* state = [[SNTSyncState alloc] init]; + state.syncType = type; + SNTConfigBundle* bundle = PostflightConfigBundle(state); + XCTAssertNotNil(bundle.fullSyncLastSuccess, + @"PostflightConfigBundle must set fullSyncLastSuccess " + @"for syncType=%lu (load-bearing for SNT-357 atomic replace).", + (unsigned long)type); + } +} + +- (void)testPostflightConfigBundleResetsSyncTypeToNormalAfterCleanSync { + SNTSyncState* cleanState = [[SNTSyncState alloc] init]; + cleanState.syncType = SNTSyncTypeClean; + SNTConfigBundle* cleanBundle = PostflightConfigBundle(cleanState); + XCTAssertEqualObjects(cleanBundle.syncType, @(SNTSyncTypeNormal)); + + SNTSyncState* cleanAllState = [[SNTSyncState alloc] init]; + cleanAllState.syncType = SNTSyncTypeCleanAll; + SNTConfigBundle* cleanAllBundle = PostflightConfigBundle(cleanAllState); + XCTAssertEqualObjects(cleanAllBundle.syncType, @(SNTSyncTypeNormal)); + + SNTSyncState* normalState = [[SNTSyncState alloc] init]; + normalState.syncType = SNTSyncTypeNormal; + SNTConfigBundle* normalBundle = PostflightConfigBundle(normalState); + XCTAssertNil(normalBundle.syncType); +} + @end diff --git a/Source/santasyncservice/SNTSyncManager.h b/Source/santasyncservice/SNTSyncManager.h index 4267df855..512577c7a 100644 --- a/Source/santasyncservice/SNTSyncManager.h +++ b/Source/santasyncservice/SNTSyncManager.h @@ -61,9 +61,15 @@ /// reply block will be called again with a SNTSyncStatusType when the sync has completed or /// failed. /// -/// Pass true to isClean to perform a clean sync, defaults to false. -/// -- (void)syncType:(SNTSyncType)syncType withReply:(void (^)(SNTSyncStatusType))reply; +/// Pass `syncType` to control rule cleanup: `SNTSyncTypeNormal` for a regular +/// sync, `SNTSyncTypeClean` to remove non-transitive rules, or +/// `SNTSyncTypeCleanAll` to remove all rules. Pass `keepOldSettings = YES` +/// on a clean sync to skip the SNT-357 settings clear at postflight (legacy +/// per-key merge semantics); on Normal syncs the flag has no effect. +/// +- (void)syncType:(SNTSyncType)syncType + keepOldSettings:(BOOL)keepOldSettings + withReply:(void (^)(SNTSyncStatusType))reply; /// /// Handle SNTSyncServiceXPC messages forwarded from SNTSyncService. diff --git a/Source/santasyncservice/SNTSyncManager.mm b/Source/santasyncservice/SNTSyncManager.mm index 62708dff2..46f187796 100644 --- a/Source/santasyncservice/SNTSyncManager.mm +++ b/Source/santasyncservice/SNTSyncManager.mm @@ -113,7 +113,7 @@ - (instancetype)initWithDaemonConnection:(MOLXPCConnection*)daemonConn { NSUInteger interval = self.pushNotifications ? self.pushNotifications.fullSyncInterval : self.persistedFullSyncInterval; [self rescheduleTimerQueue:self.fullSyncTimer secondsFromNow:interval]; - [self syncType:SNTSyncTypeNormal withReply:NULL]; + [self syncType:SNTSyncTypeNormal keepOldSettings:NO withReply:NULL]; }]; _ruleSyncTimer = [self createSyncTimerWithBlock:^{ dispatch_source_set_timer(self.ruleSyncTimer, DISPATCH_TIME_FOREVER, DISPATCH_TIME_FOREVER, @@ -326,7 +326,9 @@ - (void)syncSecondsFromNow:(uint64_t)seconds { [self rescheduleTimerQueue:self.fullSyncTimer secondsFromNow:seconds]; } -- (void)syncType:(SNTSyncType)syncType withReply:(void (^)(SNTSyncStatusType))reply { +- (void)syncType:(SNTSyncType)syncType + keepOldSettings:(BOOL)keepOldSettings + withReply:(void (^)(SNTSyncStatusType))reply { if (dispatch_semaphore_wait(self.syncLimiter, DISPATCH_TIME_NOW)) { if (reply) reply(SNTSyncStatusTypeTooManySyncsInProgress); return; @@ -346,7 +348,7 @@ - (void)syncType:(SNTSyncType)syncType withReply:(void (^)(SNTSyncStatusType))re } } if (reply) reply(SNTSyncStatusTypeSyncStarted); - SNTSyncStatusType status = [self preflight]; + SNTSyncStatusType status = [self preflightWithKeepOldSettings:keepOldSettings]; if (reply) reply(status); dispatch_semaphore_signal(self.syncLimiter); }); @@ -520,11 +522,16 @@ - (void)eventUploadForPath:(NSString*)path reply:(void (^)(NSError* error))reply #pragma mark syncing chain - (SNTSyncStatusType)preflight { + return [self preflightWithKeepOldSettings:NO]; +} + +- (SNTSyncStatusType)preflightWithKeepOldSettings:(BOOL)keepOldSettings { SNTSyncStatusType status = SNTSyncStatusTypeUnknown; SNTSyncState* syncState = [self createSyncStateWithStatus:&status]; if (!syncState) { return status; } + syncState.keepOldSettings = keepOldSettings; return [self preflightWithSyncState:syncState]; } diff --git a/Source/santasyncservice/SNTSyncPostflight.mm b/Source/santasyncservice/SNTSyncPostflight.mm index c686db208..3dfbbbf00 100644 --- a/Source/santasyncservice/SNTSyncPostflight.mm +++ b/Source/santasyncservice/SNTSyncPostflight.mm @@ -58,9 +58,18 @@ BOOL Postflight(SNTSyncPostflight* self) { typename Traits::PostflightResponseT response; [self performRequest:[self requestWithMessage:req] intoMessage:&response timeout:30]; - [rop updateSyncSettings:PostflightConfigBundle(self.syncState) - reply:^{ - }]; + SNTConfigBundle* bundle = PostflightConfigBundle(self.syncState); + if ((self.syncState.syncType == SNTSyncTypeClean || + self.syncState.syncType == SNTSyncTypeCleanAll) && + !self.syncState.keepOldSettings) { + [rop replaceSyncSettings:bundle + reply:^{ + }]; + } else { + [rop updateSyncSettings:bundle + reply:^{ + }]; + } return YES; } diff --git a/Source/santasyncservice/SNTSyncService.mm b/Source/santasyncservice/SNTSyncService.mm index 95663f748..ba390fec3 100644 --- a/Source/santasyncservice/SNTSyncService.mm +++ b/Source/santasyncservice/SNTSyncService.mm @@ -129,6 +129,7 @@ - (void)checkSyncServerStatus:(NSXPCListenerEndpoint*)logListener - (void)syncWithLogListener:(NSXPCListenerEndpoint*)logListener syncType:(SNTSyncType)syncType + keepOldSettings:(BOOL)keepOldSettings reply:(void (^)(SNTSyncStatusType))reply { MOLXPCConnection* ll; if (logListener) { @@ -138,6 +139,7 @@ - (void)syncWithLogListener:(NSXPCListenerEndpoint*)logListener [ll resume]; } [self.syncManager syncType:syncType + keepOldSettings:keepOldSettings withReply:^(SNTSyncStatusType status) { if (status == SNTSyncStatusTypeSyncStarted) { if (ll) [[SNTSyncBroadcaster broadcaster] addLogListener:ll]; diff --git a/Source/santasyncservice/SNTSyncState.h b/Source/santasyncservice/SNTSyncState.h index 9e420dd53..6499bd3e5 100644 --- a/Source/santasyncservice/SNTSyncState.h +++ b/Source/santasyncservice/SNTSyncState.h @@ -101,6 +101,13 @@ /// Clean sync flag, if True, all existing rules should be deleted before inserting any new rules. @property SNTSyncType syncType; +/// When YES, postflight uses the existing `updateSyncSettings:` path (per-key +/// merge) instead of the SNT-357 atomic `replaceSyncSettings:` path. Set only +/// via the santactl XPC entry when the operator passes `--keep-old-settings` +/// alongside `--clean[-all]`. Internal triggers (timer, push notification) +/// leave it at NO. +@property BOOL keepOldSettings; + /// Batch size for uploading events. @property NSUInteger eventBatchSize; diff --git a/Source/santasyncservice/SNTSyncTest.mm b/Source/santasyncservice/SNTSyncTest.mm index bf9f21365..e741ff928 100644 --- a/Source/santasyncservice/SNTSyncTest.mm +++ b/Source/santasyncservice/SNTSyncTest.mm @@ -1165,6 +1165,58 @@ - (void)testPostflightBasicResponse { OCMVerify([self.daemonConnRop updateSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); } +- (void)testPostflightCleanSyncRoutesToReplaceSyncSettings { + [self setupDefaultDaemonConnResponses]; + self.syncState.syncType = SNTSyncTypeClean; + self.syncState.keepOldSettings = NO; + + SNTSyncPostflight* sut = [[SNTSyncPostflight alloc] initWithState:self.syncState]; + + [self stubRequestBody:nil response:nil error:nil validateBlock:nil]; + + XCTAssertTrue([sut sync]); + OCMVerify([self.daemonConnRop replaceSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); +} + +- (void)testPostflightCleanAllSyncRoutesToReplaceSyncSettings { + [self setupDefaultDaemonConnResponses]; + self.syncState.syncType = SNTSyncTypeCleanAll; + self.syncState.keepOldSettings = NO; + + SNTSyncPostflight* sut = [[SNTSyncPostflight alloc] initWithState:self.syncState]; + + [self stubRequestBody:nil response:nil error:nil validateBlock:nil]; + + XCTAssertTrue([sut sync]); + OCMVerify([self.daemonConnRop replaceSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); +} + +- (void)testPostflightCleanSyncWithKeepOldSettingsRoutesToUpdateSyncSettings { + [self setupDefaultDaemonConnResponses]; + self.syncState.syncType = SNTSyncTypeClean; + self.syncState.keepOldSettings = YES; + + SNTSyncPostflight* sut = [[SNTSyncPostflight alloc] initWithState:self.syncState]; + + [self stubRequestBody:nil response:nil error:nil validateBlock:nil]; + + XCTAssertTrue([sut sync]); + OCMVerify([self.daemonConnRop updateSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); +} + +- (void)testPostflightCleanAllSyncWithKeepOldSettingsRoutesToUpdateSyncSettings { + [self setupDefaultDaemonConnResponses]; + self.syncState.syncType = SNTSyncTypeCleanAll; + self.syncState.keepOldSettings = YES; + + SNTSyncPostflight* sut = [[SNTSyncPostflight alloc] initWithState:self.syncState]; + + [self stubRequestBody:nil response:nil error:nil validateBlock:nil]; + + XCTAssertTrue([sut sync]); + OCMVerify([self.daemonConnRop updateSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); +} + #pragma mark - Dynamic NATS Push Client Lifecycle Tests - (void)testPreflightCreatesNATSWhenSyncV2 { From be8f63b13de886700dc0723960395d4af42ac76d Mon Sep 17 00:00:00 2001 From: Sharvil Shah Date: Wed, 6 May 2026 03:20:24 +0530 Subject: [PATCH 2/6] simplify and address feedback --- Source/common/SNTConfigurator.h | 21 +-- Source/common/SNTConfigurator.mm | 131 +----------------- Source/common/SNTConfiguratorTest.mm | 110 +++------------ Source/common/SNTXPCControlInterface.h | 14 +- Source/common/SNTXPCSyncServiceInterface.h | 1 - Source/gui/SNTAboutWindowView.swift | 6 +- Source/gui/SNTStatusItemManager.mm | 1 - Source/santactl/Commands/SNTCommandSync.mm | 37 ++--- Source/santad/SNTDaemonControlController.mm | 18 ++- .../santasyncservice/SNTSyncConfigBundle.mm | 8 ++ .../SNTSyncConfigBundleTest.mm | 22 +++ Source/santasyncservice/SNTSyncManager.h | 11 +- Source/santasyncservice/SNTSyncManager.mm | 13 +- Source/santasyncservice/SNTSyncPostflight.mm | 15 +- Source/santasyncservice/SNTSyncService.mm | 2 - Source/santasyncservice/SNTSyncState.h | 7 - Source/santasyncservice/SNTSyncTest.mm | 52 ------- 17 files changed, 103 insertions(+), 366 deletions(-) diff --git a/Source/common/SNTConfigurator.h b/Source/common/SNTConfigurator.h index 698c3c6b0..b90585a4e 100644 --- a/Source/common/SNTConfigurator.h +++ b/Source/common/SNTConfigurator.h @@ -1149,23 +1149,16 @@ extern NSString* _Nonnull const kEnableMenuItemUserOverride; #endif /// -/// Clear the sync server configuration from the effective configuration. +/// Clear the persisted sync-managed state. The in-memory dictionary is +/// reset and `sync-state.plist` is removed from disk. Called from two +/// places: (a) `SNTSyncdQueue` after `SyncBaseURL` has been removed for +/// 10 minutes, to forget settings from the previous server; and +/// (b) the daemon's `updateSyncSettings:` handler at the start of a +/// clean-sync postflight, so settings the server stops sending no longer +/// linger on disk after the bundle is applied. /// - (void)clearSyncState; -/// -/// Atomically replace the persisted sync state with a new dictionary -/// containing only the existing `PushTokenChain` (if present) plus any -/// non-nil values from `bundle`. Performs a single `saveSyncStateToDisk` -/// via `writeToFile:atomically:YES`. -/// -/// Used by the daemon's `replaceSyncSettings:reply:` XPC handler when a -/// clean sync's postflight is committing settings under the SNT-357 -/// semantics. Workshop-managed keys not present in `bundle` are -/// cleared from disk by this call. -/// -- (void)replaceSyncStateWithBundle:(nonnull SNTConfigBundle*)bundle; - /// /// Validate the configuration profile. /// diff --git a/Source/common/SNTConfigurator.mm b/Source/common/SNTConfigurator.mm index 35adf64d4..4471a942d 100644 --- a/Source/common/SNTConfigurator.mm +++ b/Source/common/SNTConfigurator.mm @@ -1959,17 +1959,11 @@ - (NSDictionary*)extraMetricLabels { /// immediately after the call completes. /// - (void)updateSyncStateForKey:(NSString*)key value:(id)value { - [self updateSyncStateForKey:key value:value writeToDisk:YES]; -} - -- (void)updateSyncStateForKey:(NSString*)key value:(id)value writeToDisk:(BOOL)writeToDisk { void (^block)(void) = ^{ NSMutableDictionary* syncState = self.syncState.mutableCopy; syncState[key] = value; self.syncState = syncState; - if (writeToDisk) { - [self saveSyncStateToDisk]; - } + [self saveSyncStateToDisk]; }; // Avoid deadlocks by directly calling the block when already running on the // main thread. @@ -2066,26 +2060,10 @@ - (void)saveSyncStateToDisk { } - (void)clearSyncState { - self.syncState = [NSMutableDictionary dictionary]; - [[NSFileManager defaultManager] removeItemAtPath:self.syncStateFilePath error:NULL]; -} - -- (void)replaceSyncStateWithBundle:(SNTConfigBundle*)bundle { void (^block)(void) = ^{ - NSMutableDictionary* newSyncState = [NSMutableDictionary dictionary]; - - NSArray* preservedChain = self.syncState[kPushTokenChainKey]; - if ([preservedChain isKindOfClass:[NSArray class]] && preservedChain.count > 0) { - newSyncState[kPushTokenChainKey] = preservedChain; - } - - self.syncState = newSyncState; - - [self applySyncBundleSettersDeferringDiskWrites:bundle]; - - [self saveSyncStateToDisk]; + self.syncState = [NSMutableDictionary dictionary]; + [[NSFileManager defaultManager] removeItemAtPath:self.syncStateFilePath error:NULL]; }; - if ([NSThread isMainThread]) { block(); } else { @@ -2093,109 +2071,6 @@ - (void)replaceSyncStateWithBundle:(SNTConfigBundle*)bundle { } } -- (void)applySyncBundleSettersDeferringDiskWrites:(SNTConfigBundle*)bundle { - [bundle clientMode:^(SNTClientMode m) { - [self updateSyncStateForKey:kClientModeKey value:@(m) writeToDisk:NO]; - }]; - [bundle syncType:^(SNTSyncType val) { - [self updateSyncStateForKey:kSyncTypeRequired value:@(val) writeToDisk:NO]; - }]; - [bundle allowlistRegex:^(NSString* val) { - [self updateSyncStateForKey:kAllowedPathRegexKey - value:[NSRegularExpression regularExpressionWithPattern:val - options:0 - error:NULL] - writeToDisk:NO]; - }]; - [bundle blocklistRegex:^(NSString* val) { - [self updateSyncStateForKey:kBlockedPathRegexKey - value:[NSRegularExpression regularExpressionWithPattern:val - options:0 - error:NULL] - writeToDisk:NO]; - }]; - [bundle removableMediaAction:^(NSString* val) { - [self updateSyncStateForKey:kRemovableMediaActionKey value:val writeToDisk:NO]; - }]; - [bundle removableMediaRemountFlags:^(NSArray* val) { - [self updateSyncStateForKey:kRemovableMediaRemountFlagsKey value:val writeToDisk:NO]; - }]; - [bundle encryptedRemovableMediaAction:^(NSString* val) { - [self updateSyncStateForKey:kEncryptedRemovableMediaActionKey value:val writeToDisk:NO]; - }]; - [bundle encryptedRemovableMediaRemountFlags:^(NSArray* val) { - [self updateSyncStateForKey:kEncryptedRemovableMediaRemountFlagsKey value:val writeToDisk:NO]; - }]; - [bundle blockNetworkMount:^(BOOL val) { - [self updateSyncStateForKey:kBlockNetworkMountKey value:@(val) writeToDisk:NO]; - }]; - [bundle bannedNetworkMountBlockMessage:^(NSString* val) { - [self updateSyncStateForKey:kBannedNetworkMountBlockMessage value:val writeToDisk:NO]; - }]; - [bundle allowedNetworkMountHosts:^(NSArray* val) { - [self updateSyncStateForKey:kAllowedNetworkMountHosts value:val writeToDisk:NO]; - }]; - [bundle enableBundles:^(BOOL val) { - [self updateSyncStateForKey:kEnableBundlesKey value:@(val) writeToDisk:NO]; - }]; - [bundle enableTransitiveRules:^(BOOL val) { - [self updateSyncStateForKey:kEnableTransitiveRulesKey value:@(val) writeToDisk:NO]; - }]; - [bundle enableAllEventUpload:^(BOOL val) { - [self updateSyncStateForKey:kEnableAllEventUploadKey value:@(val) writeToDisk:NO]; - }]; - [bundle disableUnknownEventUpload:^(BOOL val) { - [self updateSyncStateForKey:kDisableUnknownEventUploadKey value:@(val) writeToDisk:NO]; - }]; - [bundle overrideFileAccessAction:^(NSString* val) { - [self updateSyncStateForKey:kOverrideFileAccessActionKey value:val writeToDisk:NO]; - }]; - [bundle exportConfiguration:^(SNTExportConfiguration* val) { - [self updateSyncStateForKey:kExportConfigurationKey value:[val serialize] writeToDisk:NO]; - }]; - [bundle fullSyncLastSuccess:^(NSDate* val) { - [self updateSyncStateForKey:kFullSyncLastSuccess value:val writeToDisk:NO]; - }]; - [bundle ruleSyncLastSuccess:^(NSDate* val) { - [self updateSyncStateForKey:kRuleSyncLastSuccess value:val writeToDisk:NO]; - }]; - [bundle modeTransition:^(SNTModeTransition* val) { - [self updateSyncStateForKey:kModeTransitionKey value:[val serialize] writeToDisk:NO]; - }]; - [bundle networkExtensionSettings:^(SNTSyncNetworkExtensionSettings* val) { - [self updateSyncStateForKey:kNetworkExtensionSettingsKey value:[val serialize] writeToDisk:NO]; - }]; - [bundle pushTokenChain:^(NSArray* val) { - [self updateSyncStateForKey:kPushTokenChainKey value:val writeToDisk:NO]; - }]; - [bundle telemetryFilterExpressions:^(NSArray* val) { - [self updateSyncStateForKey:kTelemetryFilterExpressionsKey value:val writeToDisk:NO]; - }]; - [bundle celFallbackRules:^(NSArray* val) { - [self updateSyncStateForKey:kCELFallbackRulesKey - value:[SNTCELFallbackRule serializeArray:val] - writeToDisk:NO]; - }]; - [bundle eventDetailURL:^(NSString* val) { - [self updateSyncStateForKey:kEventDetailURLKey value:val writeToDisk:NO]; - }]; - [bundle eventDetailText:^(NSString* val) { - [self updateSyncStateForKey:kEventDetailTextKey value:val writeToDisk:NO]; - }]; - [bundle fileAccessEventDetailURL:^(NSString* val) { - [self updateSyncStateForKey:kFileAccessEventDetailURLKey value:val writeToDisk:NO]; - }]; - [bundle fileAccessEventDetailText:^(NSString* val) { - [self updateSyncStateForKey:kFileAccessEventDetailTextKey value:val writeToDisk:NO]; - }]; - [bundle fullSyncInterval:^(NSUInteger val) { - [self updateSyncStateForKey:kFullSyncInterval value:val ? @(val) : nil writeToDisk:NO]; - }]; - [bundle pushNotificationsFullSyncInterval:^(NSUInteger val) { - [self updateSyncStateForKey:kFCMFullSyncInterval value:val ? @(val) : nil writeToDisk:NO]; - }]; -} - - (NSArray*)entitlementsPrefixFilter { return EnsureArrayOfStrings(self.configState[kEntitlementsPrefixFilterKey]); } diff --git a/Source/common/SNTConfiguratorTest.mm b/Source/common/SNTConfiguratorTest.mm index 277032ca0..7a3467c27 100644 --- a/Source/common/SNTConfiguratorTest.mm +++ b/Source/common/SNTConfiguratorTest.mm @@ -295,7 +295,7 @@ - (void)testAllowDelegatedSignalsOverride { XCTAssertFalse(sut.allowDelegatedSignals); } -#pragma mark - replaceSyncStateWithBundle: tests +#pragma mark - clearSyncState tests - (SNTConfigurator*)configuratorWithSyncState:(NSDictionary*)initialState plistPath:(NSString*)plistPath { @@ -313,96 +313,6 @@ - (SNTConfigurator*)configuratorWithSyncState:(NSDictionary*)initialState }]; } -- (void)testReplaceSyncStateWithBundleClearsWorkshopKeysPreservingPushTokenChain { - NSString* plistPath = [NSString stringWithFormat:@"%@/test-replace-state.plist", self.testDir]; - NSArray* chain = @[ @"issuer-jwt-value", @"device-jwt-value" ]; - - SNTConfigurator* cfg = [self configuratorWithSyncState:@{ - @"ClientMode" : @(SNTClientModeLockdown), - @"AllowedPathRegex" : @"old-allow-pattern", - @"PushTokenChain" : chain, - } - plistPath:plistPath]; - - SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; - bundle.clientMode = @(SNTClientModeMonitor); - bundle.fullSyncLastSuccess = [NSDate date]; - - [cfg replaceSyncStateWithBundle:bundle]; - - NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; - XCTAssertEqualObjects(onDisk[@"PushTokenChain"], chain); - XCTAssertEqual([onDisk[@"ClientMode"] integerValue], SNTClientModeMonitor); - XCTAssertNotNil(onDisk[@"FullSyncLastSuccess"]); - XCTAssertNil(onDisk[@"AllowedPathRegex"]); - - XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); -} - -- (void)testReplaceSyncStateWithBundleNoExistingChainProducesNoChainOnDisk { - NSString* plistPath = [NSString stringWithFormat:@"%@/test-replace-state.plist", self.testDir]; - - SNTConfigurator* cfg = [self configuratorWithSyncState:@{ - @"ClientMode" : @(SNTClientModeLockdown), - } - plistPath:plistPath]; - - SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; - bundle.clientMode = @(SNTClientModeMonitor); - - [cfg replaceSyncStateWithBundle:bundle]; - - NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; - XCTAssertNil(onDisk[@"PushTokenChain"]); - XCTAssertEqual([onDisk[@"ClientMode"] integerValue], SNTClientModeMonitor); - - XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); -} - -- (void)testReplaceSyncStateWithBundleEmptyBundleLeavesOnlyChain { - NSString* plistPath = [NSString stringWithFormat:@"%@/test-replace-state.plist", self.testDir]; - NSArray* chain = @[ @"issuer", @"device" ]; - - SNTConfigurator* cfg = [self configuratorWithSyncState:@{ - @"ClientMode" : @(SNTClientModeLockdown), - @"PushTokenChain" : chain, - } - plistPath:plistPath]; - - SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; - - [cfg replaceSyncStateWithBundle:bundle]; - - NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; - XCTAssertEqualObjects(onDisk[@"PushTokenChain"], chain); - XCTAssertEqual(onDisk.count, (NSUInteger)1); - - XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); -} - -- (void)testReplaceSyncStateWithBundleBundleChainOverridesPreserved { - NSString* plistPath = [NSString stringWithFormat:@"%@/test-replace-state.plist", self.testDir]; - NSArray* staleChain = @[ @"old-issuer", @"old-device" ]; - NSArray* freshChain = @[ @"new-issuer", @"new-device" ]; - - SNTConfigurator* cfg = [self configuratorWithSyncState:@{ - @"PushTokenChain" : staleChain, - } - plistPath:plistPath]; - - SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; - bundle.pushTokenChain = freshChain; - - [cfg replaceSyncStateWithBundle:bundle]; - - NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; - XCTAssertEqualObjects(onDisk[@"PushTokenChain"], freshChain); - - XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); -} - -#pragma mark - clearSyncState tests (Bug 2) - - (void)testClearSyncStateRemovesDiskFile { NSString* plistPath = [NSString stringWithFormat:@"%@/test-clear-state.plist", self.testDir]; NSDictionary* contents = @{@"ClientMode" : @(SNTClientModeLockdown)}; @@ -441,4 +351,22 @@ - (void)testClearSyncStateIdempotent { XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]); } +- (void)testClearSyncStateFollowedBySetterPersistsOnlyNewValue { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-clear-then-set.plist", self.testDir]; + SNTConfigurator* cfg = [self configuratorWithSyncState:@{ + @"ClientMode" : @(SNTClientModeLockdown), + @"AllowedPathRegex" : @"old-pattern", + } + plistPath:plistPath]; + + [cfg clearSyncState]; + [cfg setSyncServerClientMode:SNTClientModeMonitor]; + + NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; + XCTAssertEqual([onDisk[@"ClientMode"] integerValue], SNTClientModeMonitor); + XCTAssertNil(onDisk[@"AllowedPathRegex"]); + + XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); +} + @end diff --git a/Source/common/SNTXPCControlInterface.h b/Source/common/SNTXPCControlInterface.h index 2e2b3febb..8c877db76 100644 --- a/Source/common/SNTXPCControlInterface.h +++ b/Source/common/SNTXPCControlInterface.h @@ -55,15 +55,15 @@ typedef NS_ENUM(NSInteger, SNTRuleAddSource) { /// /// Config ops /// -- (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply; - +/// Apply `bundle` to the persisted sync state. Non-nil values are merged +/// per-key into the existing state. /// -/// Atomically replace the persisted sync state with the contents of `bundle`, -/// preserving only `PushTokenChain` from the previous state. Used by the -/// sync service's postflight stage when a clean sync (Clean or CleanAll) -/// resolves and the user did not pass `--keep-old-settings`. +/// When `bundle.syncType == @(Normal)` (the marker `PostflightConfigBundle` +/// sets only when reverting from Clean/CleanAll), the daemon first calls +/// `clearSyncState` to wipe persisted sync state before applying the +/// bundle so settings the server stops sending no longer linger. /// -- (void)replaceSyncSettings:(SNTConfigBundle*)bundle reply:(void (^)(void))reply; +- (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply; /// /// Syncd Ops diff --git a/Source/common/SNTXPCSyncServiceInterface.h b/Source/common/SNTXPCSyncServiceInterface.h index 562d9f36f..db8a0747b 100644 --- a/Source/common/SNTXPCSyncServiceInterface.h +++ b/Source/common/SNTXPCSyncServiceInterface.h @@ -49,7 +49,6 @@ // - (void)syncWithLogListener:(NSXPCListenerEndpoint*)logListener syncType:(SNTSyncType)syncType - keepOldSettings:(BOOL)keepOldSettings reply:(void (^)(SNTSyncStatusType))reply; // Publish metrics to the sync server. The metrics dictionary is the output of SNTMetricSet export. diff --git a/Source/gui/SNTAboutWindowView.swift b/Source/gui/SNTAboutWindowView.swift index 17d8cf27e..f7d5766a1 100644 --- a/Source/gui/SNTAboutWindowView.swift +++ b/Source/gui/SNTAboutWindowView.swift @@ -206,11 +206,7 @@ struct SyncButtonView: View { lr?.resume() let proxy = ss?.remoteObjectProxy as? SNTSyncServiceXPC - proxy?.sync( - withLogListener: logListener.endpoint, - syncType: clean ? .clean : .normal, - keepOldSettings: false - ) { status in + proxy?.sync(withLogListener: logListener.endpoint, syncType: clean ? .clean : .normal) { status in lr = nil DispatchQueue.main.sync { diff --git a/Source/gui/SNTStatusItemManager.mm b/Source/gui/SNTStatusItemManager.mm index 920393ca1..b08167b65 100644 --- a/Source/gui/SNTStatusItemManager.mm +++ b/Source/gui/SNTStatusItemManager.mm @@ -362,7 +362,6 @@ - (void)syncMenuItemClicked:(id)sender { [[ss synchronousRemoteObjectProxy] syncWithLogListener:nil syncType:SNTSyncTypeNormal - keepOldSettings:NO reply:^(SNTSyncStatusType status) { dispatch_async(dispatch_get_main_queue(), ^{ self.syncMenuItem.target = self; diff --git a/Source/santactl/Commands/SNTCommandSync.mm b/Source/santactl/Commands/SNTCommandSync.mm index 8cec9d4d2..1c4feffb4 100644 --- a/Source/santactl/Commands/SNTCommandSync.mm +++ b/Source/santactl/Commands/SNTCommandSync.mm @@ -51,14 +51,10 @@ + (NSString*)longHelpText { @"this is the command used for syncing.\n\n" @"Options:\n" @" --clean: Perform a clean sync, erasing all existing non-transitive rules and\n" - @" requesting a clean sync from the server. Also clears Workshop-managed\n" - @" sync settings; pass --keep-old-settings to retain them.\n" + @" requesting a clean sync from the server. Also clears sync-managed\n" + @" settings.\n" @" --clean-all: Perform a clean sync, erasing all existing rules and requesting a\n" - @" clean sync from the server. Also clears Workshop-managed sync\n" - @" settings; pass --keep-old-settings to retain them.\n" - @" --keep-old-settings: Only valid with --clean or --clean-all. Preserve existing\n" - @" Workshop-managed sync settings on disk during the clean\n" - @" sync, instead of clearing them.\n" + @" clean sync from the server. Also clears sync-managed settings.\n" @" --debug: Enable verbose output.\n"); } @@ -73,23 +69,6 @@ - (void)runWithArguments:(NSArray*)arguments { TEE_LOGE(@"Missing SyncBaseURL. Exiting."); exit(1); } - - NSArray* args = NSProcessInfo.processInfo.arguments; - SNTSyncType syncType = SNTSyncTypeNormal; - if ([args containsObject:@"--clean-all"]) { - syncType = SNTSyncTypeCleanAll; - } else if ([args containsObject:@"--clean"]) { - syncType = SNTSyncTypeClean; - } - - BOOL keepOldSettings = [args containsObject:@"--keep-old-settings"]; - if (keepOldSettings && syncType == SNTSyncTypeNormal) { - TEE_LOGE(@"--keep-old-settings requires --clean or --clean-all. Exiting."); - exit(1); - } - - self.enableDebugLogging = [args containsObject:@"--debug"]; - MOLXPCConnection* ss = [SNTXPCSyncServiceInterface configuredConnection]; ss.invalidationHandler = ^(void) { TEE_LOGE(@"Failed to connect to the sync service."); @@ -104,10 +83,18 @@ - (void)runWithArguments:(NSArray*)arguments { [NSXPCInterface interfaceWithProtocol:@protocol(SNTSyncServiceLogReceiverXPC)]; [lr resume]; + SNTSyncType syncType = SNTSyncTypeNormal; + if ([NSProcessInfo.processInfo.arguments containsObject:@"--clean-all"]) { + syncType = SNTSyncTypeCleanAll; + } else if ([NSProcessInfo.processInfo.arguments containsObject:@"--clean"]) { + syncType = SNTSyncTypeClean; + } + + self.enableDebugLogging = [NSProcessInfo.processInfo.arguments containsObject:@"--debug"]; + [[ss remoteObjectProxy] syncWithLogListener:logListener.endpoint syncType:syncType - keepOldSettings:keepOldSettings reply:^(SNTSyncStatusType status) { if (status == SNTSyncStatusTypeTooManySyncsInProgress) { [self didReceiveLog:@"Too many syncs in progress, try again later." diff --git a/Source/santad/SNTDaemonControlController.mm b/Source/santad/SNTDaemonControlController.mm index 5aaf9b0f2..af97a2051 100644 --- a/Source/santad/SNTDaemonControlController.mm +++ b/Source/santad/SNTDaemonControlController.mm @@ -473,6 +473,19 @@ - (void)disableUnknownEventUpload:(void (^)(BOOL))reply { - (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply { SNTConfigurator* configurator = [SNTConfigurator configurator]; + // PostflightConfigBundle sets the bundle's syncType to Normal only when + // the in-flight sync was Clean or CleanAll (the directive is consumed by + // resetting it on disk). That exact transition is the signal that this + // bundle is a clean-sync postflight: wipe persisted sync state before + // applying so settings the server stops sending don't linger. The + // per-key setters that follow recreate `sync-state.plist` from the + // bundle's contents. + [result syncType:^(SNTSyncType val) { + if (val == SNTSyncTypeNormal) { + [configurator clearSyncState]; + } + }]; + [result clientMode:^(SNTClientMode m) { [configurator setSyncServerClientMode:m]; }]; @@ -609,11 +622,6 @@ - (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply reply(); } -- (void)replaceSyncSettings:(SNTConfigBundle*)bundle reply:(void (^)(void))reply { - [[SNTConfigurator configurator] replaceSyncStateWithBundle:bundle]; - reply(); -} - - (void)retrieveStatsState:(void (^)(NSDate*, NSString*))reply { reply([[SNTConfigurator configurator] lastStatsSubmissionTimestamp], [[SNTConfigurator configurator] lastStatsSubmissionVersion]); diff --git a/Source/santasyncservice/SNTSyncConfigBundle.mm b/Source/santasyncservice/SNTSyncConfigBundle.mm index 9781b0e88..4e11ba688 100644 --- a/Source/santasyncservice/SNTSyncConfigBundle.mm +++ b/Source/santasyncservice/SNTSyncConfigBundle.mm @@ -68,6 +68,10 @@ @interface SNTConfigBundle (ConfigBundleCreator) SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; bundle.clientMode = syncState.clientMode ? @(syncState.clientMode) : nil; + // Clean syncs reset SyncTypeRequired to Normal. The daemon recognizes + // this exact transition (`syncType == @(Normal)`) as the signal that + // this is a clean-sync postflight bundle and clears persisted sync + // state before applying. bundle.syncType = syncState.syncType != SNTSyncTypeNormal ? @(SNTSyncTypeNormal) : nil; bundle.allowlistRegex = syncState.allowlistRegex; bundle.blocklistRegex = syncState.blocklistRegex; @@ -95,6 +99,10 @@ @interface SNTConfigBundle (ConfigBundleCreator) bundle.fullSyncInterval = syncState.fullSyncInterval; bundle.pushNotificationsFullSyncInterval = syncState.pushNotificationsFullSyncInterval; + if (syncState.pushIssuerJWT.length && syncState.pushJWT.length) { + bundle.pushTokenChain = @[ syncState.pushIssuerJWT, syncState.pushJWT ]; + } + bundle.fullSyncLastSuccess = [NSDate now]; return bundle; diff --git a/Source/santasyncservice/SNTSyncConfigBundleTest.mm b/Source/santasyncservice/SNTSyncConfigBundleTest.mm index 79dd60508..f464c7220 100644 --- a/Source/santasyncservice/SNTSyncConfigBundleTest.mm +++ b/Source/santasyncservice/SNTSyncConfigBundleTest.mm @@ -293,4 +293,26 @@ - (void)testPostflightConfigBundleResetsSyncTypeToNormalAfterCleanSync { XCTAssertNil(normalBundle.syncType); } +- (void)testPostflightConfigBundleForwardsPushTokenChain { + SNTSyncState* syncState = [[SNTSyncState alloc] init]; + syncState.pushIssuerJWT = @"issuerToken"; + syncState.pushJWT = @"userToken"; + + SNTConfigBundle* bundle = PostflightConfigBundle(syncState); + XCTAssertEqualObjects(bundle.pushTokenChain, (@[ @"issuerToken", @"userToken" ])); +} + +- (void)testPostflightConfigBundleOmitsPushTokenChainWhenJWTsMissing { + SNTSyncState* syncState = [[SNTSyncState alloc] init]; + // Both empty: no chain. + SNTConfigBundle* bundle = PostflightConfigBundle(syncState); + XCTAssertNil(bundle.pushTokenChain); + + // One missing: still no chain (defensive — server is expected to always + // send both, but this ensures we don't ship a half-chain to the daemon). + syncState.pushIssuerJWT = @"issuerToken"; + bundle = PostflightConfigBundle(syncState); + XCTAssertNil(bundle.pushTokenChain); +} + @end diff --git a/Source/santasyncservice/SNTSyncManager.h b/Source/santasyncservice/SNTSyncManager.h index 512577c7a..1586f5b88 100644 --- a/Source/santasyncservice/SNTSyncManager.h +++ b/Source/santasyncservice/SNTSyncManager.h @@ -63,13 +63,12 @@ /// /// Pass `syncType` to control rule cleanup: `SNTSyncTypeNormal` for a regular /// sync, `SNTSyncTypeClean` to remove non-transitive rules, or -/// `SNTSyncTypeCleanAll` to remove all rules. Pass `keepOldSettings = YES` -/// on a clean sync to skip the SNT-357 settings clear at postflight (legacy -/// per-key merge semantics); on Normal syncs the flag has no effect. +/// `SNTSyncTypeCleanAll` to remove all rules. On Clean and CleanAll, the +/// postflight bundle signals the daemon to wipe persisted sync-managed +/// state before applying the bundle so settings the server stops sending +/// no longer linger. /// -- (void)syncType:(SNTSyncType)syncType - keepOldSettings:(BOOL)keepOldSettings - withReply:(void (^)(SNTSyncStatusType))reply; +- (void)syncType:(SNTSyncType)syncType withReply:(void (^)(SNTSyncStatusType))reply; /// /// Handle SNTSyncServiceXPC messages forwarded from SNTSyncService. diff --git a/Source/santasyncservice/SNTSyncManager.mm b/Source/santasyncservice/SNTSyncManager.mm index 46f187796..62708dff2 100644 --- a/Source/santasyncservice/SNTSyncManager.mm +++ b/Source/santasyncservice/SNTSyncManager.mm @@ -113,7 +113,7 @@ - (instancetype)initWithDaemonConnection:(MOLXPCConnection*)daemonConn { NSUInteger interval = self.pushNotifications ? self.pushNotifications.fullSyncInterval : self.persistedFullSyncInterval; [self rescheduleTimerQueue:self.fullSyncTimer secondsFromNow:interval]; - [self syncType:SNTSyncTypeNormal keepOldSettings:NO withReply:NULL]; + [self syncType:SNTSyncTypeNormal withReply:NULL]; }]; _ruleSyncTimer = [self createSyncTimerWithBlock:^{ dispatch_source_set_timer(self.ruleSyncTimer, DISPATCH_TIME_FOREVER, DISPATCH_TIME_FOREVER, @@ -326,9 +326,7 @@ - (void)syncSecondsFromNow:(uint64_t)seconds { [self rescheduleTimerQueue:self.fullSyncTimer secondsFromNow:seconds]; } -- (void)syncType:(SNTSyncType)syncType - keepOldSettings:(BOOL)keepOldSettings - withReply:(void (^)(SNTSyncStatusType))reply { +- (void)syncType:(SNTSyncType)syncType withReply:(void (^)(SNTSyncStatusType))reply { if (dispatch_semaphore_wait(self.syncLimiter, DISPATCH_TIME_NOW)) { if (reply) reply(SNTSyncStatusTypeTooManySyncsInProgress); return; @@ -348,7 +346,7 @@ - (void)syncType:(SNTSyncType)syncType } } if (reply) reply(SNTSyncStatusTypeSyncStarted); - SNTSyncStatusType status = [self preflightWithKeepOldSettings:keepOldSettings]; + SNTSyncStatusType status = [self preflight]; if (reply) reply(status); dispatch_semaphore_signal(self.syncLimiter); }); @@ -522,16 +520,11 @@ - (void)eventUploadForPath:(NSString*)path reply:(void (^)(NSError* error))reply #pragma mark syncing chain - (SNTSyncStatusType)preflight { - return [self preflightWithKeepOldSettings:NO]; -} - -- (SNTSyncStatusType)preflightWithKeepOldSettings:(BOOL)keepOldSettings { SNTSyncStatusType status = SNTSyncStatusTypeUnknown; SNTSyncState* syncState = [self createSyncStateWithStatus:&status]; if (!syncState) { return status; } - syncState.keepOldSettings = keepOldSettings; return [self preflightWithSyncState:syncState]; } diff --git a/Source/santasyncservice/SNTSyncPostflight.mm b/Source/santasyncservice/SNTSyncPostflight.mm index 3dfbbbf00..c686db208 100644 --- a/Source/santasyncservice/SNTSyncPostflight.mm +++ b/Source/santasyncservice/SNTSyncPostflight.mm @@ -58,18 +58,9 @@ BOOL Postflight(SNTSyncPostflight* self) { typename Traits::PostflightResponseT response; [self performRequest:[self requestWithMessage:req] intoMessage:&response timeout:30]; - SNTConfigBundle* bundle = PostflightConfigBundle(self.syncState); - if ((self.syncState.syncType == SNTSyncTypeClean || - self.syncState.syncType == SNTSyncTypeCleanAll) && - !self.syncState.keepOldSettings) { - [rop replaceSyncSettings:bundle - reply:^{ - }]; - } else { - [rop updateSyncSettings:bundle - reply:^{ - }]; - } + [rop updateSyncSettings:PostflightConfigBundle(self.syncState) + reply:^{ + }]; return YES; } diff --git a/Source/santasyncservice/SNTSyncService.mm b/Source/santasyncservice/SNTSyncService.mm index ba390fec3..95663f748 100644 --- a/Source/santasyncservice/SNTSyncService.mm +++ b/Source/santasyncservice/SNTSyncService.mm @@ -129,7 +129,6 @@ - (void)checkSyncServerStatus:(NSXPCListenerEndpoint*)logListener - (void)syncWithLogListener:(NSXPCListenerEndpoint*)logListener syncType:(SNTSyncType)syncType - keepOldSettings:(BOOL)keepOldSettings reply:(void (^)(SNTSyncStatusType))reply { MOLXPCConnection* ll; if (logListener) { @@ -139,7 +138,6 @@ - (void)syncWithLogListener:(NSXPCListenerEndpoint*)logListener [ll resume]; } [self.syncManager syncType:syncType - keepOldSettings:keepOldSettings withReply:^(SNTSyncStatusType status) { if (status == SNTSyncStatusTypeSyncStarted) { if (ll) [[SNTSyncBroadcaster broadcaster] addLogListener:ll]; diff --git a/Source/santasyncservice/SNTSyncState.h b/Source/santasyncservice/SNTSyncState.h index 6499bd3e5..9e420dd53 100644 --- a/Source/santasyncservice/SNTSyncState.h +++ b/Source/santasyncservice/SNTSyncState.h @@ -101,13 +101,6 @@ /// Clean sync flag, if True, all existing rules should be deleted before inserting any new rules. @property SNTSyncType syncType; -/// When YES, postflight uses the existing `updateSyncSettings:` path (per-key -/// merge) instead of the SNT-357 atomic `replaceSyncSettings:` path. Set only -/// via the santactl XPC entry when the operator passes `--keep-old-settings` -/// alongside `--clean[-all]`. Internal triggers (timer, push notification) -/// leave it at NO. -@property BOOL keepOldSettings; - /// Batch size for uploading events. @property NSUInteger eventBatchSize; diff --git a/Source/santasyncservice/SNTSyncTest.mm b/Source/santasyncservice/SNTSyncTest.mm index e741ff928..bf9f21365 100644 --- a/Source/santasyncservice/SNTSyncTest.mm +++ b/Source/santasyncservice/SNTSyncTest.mm @@ -1165,58 +1165,6 @@ - (void)testPostflightBasicResponse { OCMVerify([self.daemonConnRop updateSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); } -- (void)testPostflightCleanSyncRoutesToReplaceSyncSettings { - [self setupDefaultDaemonConnResponses]; - self.syncState.syncType = SNTSyncTypeClean; - self.syncState.keepOldSettings = NO; - - SNTSyncPostflight* sut = [[SNTSyncPostflight alloc] initWithState:self.syncState]; - - [self stubRequestBody:nil response:nil error:nil validateBlock:nil]; - - XCTAssertTrue([sut sync]); - OCMVerify([self.daemonConnRop replaceSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); -} - -- (void)testPostflightCleanAllSyncRoutesToReplaceSyncSettings { - [self setupDefaultDaemonConnResponses]; - self.syncState.syncType = SNTSyncTypeCleanAll; - self.syncState.keepOldSettings = NO; - - SNTSyncPostflight* sut = [[SNTSyncPostflight alloc] initWithState:self.syncState]; - - [self stubRequestBody:nil response:nil error:nil validateBlock:nil]; - - XCTAssertTrue([sut sync]); - OCMVerify([self.daemonConnRop replaceSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); -} - -- (void)testPostflightCleanSyncWithKeepOldSettingsRoutesToUpdateSyncSettings { - [self setupDefaultDaemonConnResponses]; - self.syncState.syncType = SNTSyncTypeClean; - self.syncState.keepOldSettings = YES; - - SNTSyncPostflight* sut = [[SNTSyncPostflight alloc] initWithState:self.syncState]; - - [self stubRequestBody:nil response:nil error:nil validateBlock:nil]; - - XCTAssertTrue([sut sync]); - OCMVerify([self.daemonConnRop updateSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); -} - -- (void)testPostflightCleanAllSyncWithKeepOldSettingsRoutesToUpdateSyncSettings { - [self setupDefaultDaemonConnResponses]; - self.syncState.syncType = SNTSyncTypeCleanAll; - self.syncState.keepOldSettings = YES; - - SNTSyncPostflight* sut = [[SNTSyncPostflight alloc] initWithState:self.syncState]; - - [self stubRequestBody:nil response:nil error:nil validateBlock:nil]; - - XCTAssertTrue([sut sync]); - OCMVerify([self.daemonConnRop updateSyncSettings:OCMOCK_ANY reply:OCMOCK_ANY]); -} - #pragma mark - Dynamic NATS Push Client Lifecycle Tests - (void)testPreflightCreatesNATSWhenSyncV2 { From 364340ff06a3e772db187900884dd75f5d0f45ba Mon Sep 17 00:00:00 2001 From: Sharvil Shah Date: Thu, 7 May 2026 19:24:27 +0530 Subject: [PATCH 3/6] feedback and fixes --- Source/common/SNTConfigBundle.h | 1 + Source/common/SNTConfigBundle.mm | 9 ++ Source/common/SNTConfigBundleTest.mm | 55 +++++++- Source/common/SNTConfigurator.h | 21 +++- Source/common/SNTConfigurator.mm | 118 ++++++++++++++++++ Source/common/SNTConfiguratorTest.mm | 112 +++++++++++++++-- Source/common/SNTXPCControlInterface.h | 19 +-- Source/santad/SNTDaemonControlController.mm | 37 ++++-- .../santasyncservice/SNTSyncConfigBundle.mm | 5 + .../SNTSyncConfigBundleTest.mm | 49 ++++++++ Source/santasyncservice/SNTSyncManager.h | 6 +- Source/santasyncservice/SNTSyncTest.mm | 68 ++++++++++ 12 files changed, 465 insertions(+), 35 deletions(-) diff --git a/Source/common/SNTConfigBundle.h b/Source/common/SNTConfigBundle.h index c281cb1be..111608c94 100644 --- a/Source/common/SNTConfigBundle.h +++ b/Source/common/SNTConfigBundle.h @@ -55,5 +55,6 @@ - (void)celFallbackRules:(void (^)(NSArray*))block; - (void)fullSyncInterval:(void (^)(NSUInteger))block; - (void)pushNotificationsFullSyncInterval:(void (^)(NSUInteger))block; +- (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..35a7c9a7d 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 @@ -62,7 +63,7 @@ @implementation SNTConfigBundleTest - (void)testGettersWithValues { __block XCTestExpectation* exp = [self expectationWithDescription:@"Result Blocks"]; - exp.expectedFulfillmentCount = 29; + exp.expectedFulfillmentCount = 30; NSDate* nowDate = [NSDate now]; SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; @@ -97,6 +98,7 @@ - (void)testGettersWithValues { bundle.pushTokenChain = @[ @"issuerJWT", @"userJWT" ]; bundle.fullSyncInterval = @(600); bundle.pushNotificationsFullSyncInterval = @(21600); + bundle.clearSyncStateBeforeApply = @(YES); [bundle clientMode:^(SNTClientMode val) { XCTAssertEqual(val, SNTClientModeLockdown); @@ -248,6 +250,11 @@ - (void)testGettersWithValues { [exp fulfill]; }]; + [bundle clearSyncStateBeforeApply:^(BOOL val) { + XCTAssertNotEqual(val, NO); + [exp fulfill]; + }]; + // Low timeout because code above is synchronous [self waitForExpectationsWithTimeout:0.1 handler:NULL]; } @@ -370,6 +377,52 @@ - (void)testGettersWithoutValues { [bundle pushNotificationsFullSyncInterval:^(NSUInteger val) { XCTFail(@"This shouldn't be called"); }]; + + [bundle clearSyncStateBeforeApply:^(BOOL val) { + XCTFail(@"This shouldn't be called"); + }]; +} + +- (void)testClearSyncStateBeforeApplyEncodesAndDecodes { + // When set to YES, the round-trip preserves the value and the accessor block fires. + 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 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 b90585a4e..21edc22d4 100644 --- a/Source/common/SNTConfigurator.h +++ b/Source/common/SNTConfigurator.h @@ -1150,15 +1150,24 @@ extern NSString* _Nonnull const kEnableMenuItemUserOverride; /// /// Clear the persisted sync-managed state. The in-memory dictionary is -/// reset and `sync-state.plist` is removed from disk. Called from two -/// places: (a) `SNTSyncdQueue` after `SyncBaseURL` has been removed for -/// 10 minutes, to forget settings from the previous server; and -/// (b) the daemon's `updateSyncSettings:` handler at the start of a -/// clean-sync postflight, so settings the server stops sending no longer -/// linger on disk after the bundle is applied. +/// reset and `sync-state.plist` is removed from disk. Called by +/// `SNTSyncdQueue` after `SyncBaseURL` has been removed for 10 minutes, +/// to forget settings from the previous server. /// - (void)clearSyncState; +/// +/// Atomically replace the persisted sync-managed state with the bundle's contents. +/// Walks the bundle once, builds a fresh in-memory dictionary (with inline +/// validation/translation matching the existing per-key setters), then performs +/// a single in-memory swap and a single disk write. Used by the daemon's +/// `updateSyncSettings:` clean-sync branch (when `bundle.clearSyncStateBeforeApply` +/// is YES). Slots intentionally not handled: `clearSyncStateBeforeApply` itself +/// (consumed by the daemon's branching), and `modeTransition` (owned by +/// `_temporaryMonitorMode` for combined persistence + side effects). +/// +- (void)atomicallyApplyBundle:(nonnull SNTConfigBundle*)bundle; + /// /// Validate the configuration profile. /// diff --git a/Source/common/SNTConfigurator.mm b/Source/common/SNTConfigurator.mm index 4471a942d..3185ae2c3 100644 --- a/Source/common/SNTConfigurator.mm +++ b/Source/common/SNTConfigurator.mm @@ -2071,6 +2071,124 @@ - (void)clearSyncState { } } +- (void)atomicallyApplyBundle:(SNTConfigBundle*)bundle { + NSMutableDictionary* newSyncState = [NSMutableDictionary dictionary]; + + [bundle clientMode:^(SNTClientMode m) { + if (m == SNTClientModeMonitor || m == SNTClientModeLockdown || m == SNTClientModeStandalone) { + newSyncState[kClientModeKey] = @(m); + } + }]; + [bundle syncType:^(SNTSyncType v) { + newSyncState[kSyncTypeRequired] = @(v); + }]; + [bundle allowlistRegex:^(NSString* pattern) { + NSRegularExpression* re = [NSRegularExpression regularExpressionWithPattern:pattern + options:0 + error:NULL]; + if (re) { + newSyncState[kAllowedPathRegexKey] = re; + } + }]; + [bundle blocklistRegex:^(NSString* pattern) { + NSRegularExpression* re = [NSRegularExpression regularExpressionWithPattern:pattern + options:0 + error:NULL]; + if (re) { + newSyncState[kBlockedPathRegexKey] = re; + } + }]; + [bundle removableMediaAction:^(NSString* v) { + newSyncState[kRemovableMediaActionKey] = v; + }]; + [bundle removableMediaRemountFlags:^(NSArray* v) { + newSyncState[kRemovableMediaRemountFlagsKey] = v; + }]; + [bundle encryptedRemovableMediaAction:^(NSString* v) { + newSyncState[kEncryptedRemovableMediaActionKey] = v; + }]; + [bundle encryptedRemovableMediaRemountFlags:^(NSArray* v) { + newSyncState[kEncryptedRemovableMediaRemountFlagsKey] = v; + }]; + [bundle blockNetworkMount:^(BOOL v) { + newSyncState[kBlockNetworkMountKey] = @(v); + }]; + [bundle bannedNetworkMountBlockMessage:^(NSString* v) { + newSyncState[kBannedNetworkMountBlockMessage] = v; + }]; + [bundle allowedNetworkMountHosts:^(NSArray* v) { + newSyncState[kAllowedNetworkMountHosts] = v; + }]; + [bundle enableBundles:^(BOOL v) { + newSyncState[kEnableBundlesKey] = @(v); + }]; + [bundle enableTransitiveRules:^(BOOL v) { + newSyncState[kEnableTransitiveRulesKey] = @(v); + }]; + [bundle enableAllEventUpload:^(BOOL v) { + newSyncState[kEnableAllEventUploadKey] = @(v); + }]; + [bundle disableUnknownEventUpload:^(BOOL v) { + newSyncState[kDisableUnknownEventUploadKey] = @(v); + }]; + [bundle overrideFileAccessAction:^(NSString* v) { + NSString* lower = [v lowercaseString]; + if ([lower isEqualToString:@"auditonly"] || [lower isEqualToString:@"disable"] || + [lower isEqualToString:@"none"] || [lower isEqualToString:@""]) { + newSyncState[kOverrideFileAccessActionKey] = v; + } + }]; + [bundle exportConfiguration:^(SNTExportConfiguration* v) { + newSyncState[kExportConfigurationKey] = [v serialize]; + }]; + [bundle fullSyncLastSuccess:^(NSDate* v) { + newSyncState[kFullSyncLastSuccess] = v; + }]; + [bundle ruleSyncLastSuccess:^(NSDate* v) { + newSyncState[kRuleSyncLastSuccess] = v; + }]; + [bundle eventDetailURL:^(NSString* v) { + newSyncState[kEventDetailURLKey] = v; + }]; + [bundle eventDetailText:^(NSString* v) { + newSyncState[kEventDetailTextKey] = v; + }]; + [bundle fileAccessEventDetailURL:^(NSString* v) { + newSyncState[kFileAccessEventDetailURLKey] = v; + }]; + [bundle fileAccessEventDetailText:^(NSString* v) { + newSyncState[kFileAccessEventDetailTextKey] = v; + }]; + [bundle networkExtensionSettings:^(SNTSyncNetworkExtensionSettings* v) { + newSyncState[kNetworkExtensionSettingsKey] = [v serialize]; + }]; + [bundle pushTokenChain:^(NSArray* v) { + newSyncState[kPushTokenChainKey] = EnsureArrayOfStrings(v); + }]; + [bundle telemetryFilterExpressions:^(NSArray* v) { + newSyncState[kTelemetryFilterExpressionsKey] = EnsureArrayOfStrings(v); + }]; + [bundle celFallbackRules:^(NSArray* v) { + newSyncState[kCELFallbackRulesKey] = [SNTCELFallbackRule serializeArray:v]; + }]; + [bundle fullSyncInterval:^(NSUInteger v) { + newSyncState[kFullSyncInterval] = v ? @(v) : nil; + }]; + [bundle pushNotificationsFullSyncInterval:^(NSUInteger v) { + newSyncState[kFCMFullSyncInterval] = v ? @(v) : nil; + }]; + + void (^commit)(void) = ^{ + self.syncState = newSyncState; + [self saveSyncStateToDisk]; + }; + if ([NSThread isMainThread]) { + commit(); + } else { + dispatch_sync(dispatch_get_main_queue(), commit); + } +} + - (NSArray*)entitlementsPrefixFilter { return EnsureArrayOfStrings(self.configState[kEntitlementsPrefixFilterKey]); } diff --git a/Source/common/SNTConfiguratorTest.mm b/Source/common/SNTConfiguratorTest.mm index 7a3467c27..8ead25579 100644 --- a/Source/common/SNTConfiguratorTest.mm +++ b/Source/common/SNTConfiguratorTest.mm @@ -38,8 +38,31 @@ - (instancetype)initWithSyncStateFile:(NSString*)syncStateFilePath // a category here gives the test typed setter access. @interface SNTConfigBundle (ConfigBundleCreator) @property NSNumber* clientMode; +@property NSNumber* syncType; +@property NSString* allowlistRegex; +@property NSString* blocklistRegex; +@property NSString* removableMediaAction; +@property NSArray* removableMediaRemountFlags; +@property NSString* encryptedRemovableMediaAction; +@property NSArray* encryptedRemovableMediaRemountFlags; +@property NSNumber* blockNetworkMount; +@property NSString* bannedNetworkMountBlockMessage; +@property NSArray* allowedNetworkMountHosts; +@property NSNumber* enableBundles; +@property NSNumber* enableTransitiveRules; +@property NSNumber* enableAllEventUpload; +@property NSNumber* disableUnknownEventUpload; +@property NSString* overrideFileAccessAction; @property NSDate* fullSyncLastSuccess; +@property NSDate* ruleSyncLastSuccess; +@property NSString* eventDetailURL; +@property NSString* eventDetailText; +@property NSString* fileAccessEventDetailURL; +@property NSString* fileAccessEventDetailText; @property NSArray* pushTokenChain; +@property NSArray* telemetryFilterExpressions; +@property NSNumber* fullSyncInterval; +@property NSNumber* pushNotificationsFullSyncInterval; @end @interface SNTConfiguratorTest : XCTestCase @@ -351,20 +374,95 @@ - (void)testClearSyncStateIdempotent { XCTAssertFalse([self.fileMgr fileExistsAtPath:plistPath]); } -- (void)testClearSyncStateFollowedBySetterPersistsOnlyNewValue { - NSString* plistPath = [NSString stringWithFormat:@"%@/test-clear-then-set.plist", self.testDir]; +#pragma mark - atomicallyApplyBundle: tests + +- (void)testAtomicallyApplyBundleReplacesAllKeys { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-atomic-replace.plist", self.testDir]; + + // Pre-populate sync state with stale values — the atomic apply replaces all of them. SNTConfigurator* cfg = [self configuratorWithSyncState:@{ @"ClientMode" : @(SNTClientModeLockdown), @"AllowedPathRegex" : @"old-pattern", + @"OverrideFileAccessAction" : @"disable", + @"full_sync_interval" : @(99999), } plistPath:plistPath]; - [cfg clearSyncState]; - [cfg setSyncServerClientMode:SNTClientModeMonitor]; - + // Build a bundle exercising every slot atomicallyApplyBundle: handles. + SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; + // Use the testing category to set bundle slots directly. + bundle.clientMode = @(SNTClientModeMonitor); + bundle.syncType = @(SNTSyncTypeNormal); + bundle.allowlistRegex = @"new-allow"; + bundle.blocklistRegex = @"new-block"; + bundle.removableMediaAction = @"Block"; + bundle.removableMediaRemountFlags = @[ @"foo" ]; + bundle.encryptedRemovableMediaAction = @"Remount"; + bundle.encryptedRemovableMediaRemountFlags = @[ @"rdonly" ]; + bundle.blockNetworkMount = @(YES); + bundle.bannedNetworkMountBlockMessage = @"blocked"; + bundle.allowedNetworkMountHosts = @[ @"example.com" ]; + bundle.enableBundles = @(YES); + bundle.enableTransitiveRules = @(YES); + bundle.enableAllEventUpload = @(NO); + bundle.disableUnknownEventUpload = @(YES); + bundle.overrideFileAccessAction = @"AuditOnly"; // case validated; original case stored + bundle.fullSyncLastSuccess = [NSDate dateWithTimeIntervalSince1970:1000]; + bundle.ruleSyncLastSuccess = [NSDate dateWithTimeIntervalSince1970:2000]; + bundle.eventDetailURL = @"https://x/details"; + bundle.eventDetailText = @"View"; + bundle.fileAccessEventDetailURL = @"https://x/faa"; + bundle.fileAccessEventDetailText = @"View FAA"; + bundle.pushTokenChain = @[ @"issuerJWT", @"jwt" ]; + bundle.telemetryFilterExpressions = @[ @"expr" ]; + bundle.fullSyncInterval = @(600); + bundle.pushNotificationsFullSyncInterval = @(21600); + + [cfg atomicallyApplyBundle:bundle]; + + // FullSyncInterval set by the bundle replaces the stale 99999. + XCTAssertEqualObjects(cfg.syncState[@"full_sync_interval"], @(600)); + // Pre-existing stale values are replaced (not merged). + XCTAssertTrue([cfg.syncState[@"AllowedPathRegex"] isKindOfClass:[NSRegularExpression class]]); + XCTAssertEqualObjects([cfg.syncState[@"AllowedPathRegex"] pattern], @"new-allow"); + XCTAssertTrue([cfg.syncState[@"BlockedPathRegex"] isKindOfClass:[NSRegularExpression class]]); + XCTAssertEqualObjects([cfg.syncState[@"BlockedPathRegex"] pattern], @"new-block"); + + // ClientMode validated and stored. + XCTAssertEqualObjects(cfg.syncState[@"ClientMode"], @(SNTClientModeMonitor)); + + // OverrideFileAccessAction stored in original case (matching + // setSyncServerOverrideFileAccessAction:). + XCTAssertEqualObjects(cfg.syncState[@"OverrideFileAccessAction"], @"AuditOnly"); + + // Push token chain and telemetry filter expressions ensure-string-filtered. + XCTAssertEqualObjects(cfg.syncState[@"PushTokenChain"], (@[ @"issuerJWT", @"jwt" ])); + XCTAssertEqualObjects(cfg.syncState[@"TelemetryFilterExpressions"], (@[ @"expr" ])); + + // Bundle slots that the method excludes are absent. + XCTAssertNil(cfg.syncState[@"ModeTransition"]); + + // Disk file matches in-memory state (modulo regex flatten). NSDictionary* onDisk = [NSDictionary dictionaryWithContentsOfFile:plistPath]; - XCTAssertEqual([onDisk[@"ClientMode"] integerValue], SNTClientModeMonitor); - XCTAssertNil(onDisk[@"AllowedPathRegex"]); + XCTAssertEqualObjects(onDisk[@"ClientMode"], @(SNTClientModeMonitor)); + XCTAssertEqualObjects(onDisk[@"AllowedPathRegex"], @"new-allow"); // pattern string on disk + XCTAssertEqualObjects(onDisk[@"OverrideFileAccessAction"], @"AuditOnly"); + + XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); +} + +- (void)testAtomicallyApplyBundleDropsInvalidValues { + NSString* plistPath = [NSString stringWithFormat:@"%@/test-atomic-drops.plist", self.testDir]; + SNTConfigurator* cfg = [self configuratorWithSyncState:@{} plistPath:plistPath]; + + SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; + bundle.clientMode = @(42); // invalid enum + bundle.overrideFileAccessAction = @"warn"; // not in allow-list + + [cfg atomicallyApplyBundle:bundle]; + + XCTAssertNil(cfg.syncState[@"ClientMode"]); + XCTAssertNil(cfg.syncState[@"OverrideFileAccessAction"]); XCTAssertTrue([self.fileMgr removeItemAtPath:plistPath error:nil]); } diff --git a/Source/common/SNTXPCControlInterface.h b/Source/common/SNTXPCControlInterface.h index 8c877db76..16acba0d2 100644 --- a/Source/common/SNTXPCControlInterface.h +++ b/Source/common/SNTXPCControlInterface.h @@ -55,13 +55,18 @@ typedef NS_ENUM(NSInteger, SNTRuleAddSource) { /// /// Config ops /// -/// Apply `bundle` to the persisted sync state. Non-nil values are merged -/// per-key into the existing state. -/// -/// When `bundle.syncType == @(Normal)` (the marker `PostflightConfigBundle` -/// sets only when reverting from Clean/CleanAll), the daemon first calls -/// `clearSyncState` to wipe persisted sync state before applying the -/// bundle so settings the server stops sending no longer linger. +/// Apply `bundle` to the persisted sync state. By default, non-nil values +/// are merged per-key into the existing state (every other sync stage's +/// incremental write). +/// +/// When `bundle.clearSyncStateBeforeApply == YES` (set by +/// `PostflightConfigBundle` only when the in-flight sync was Clean or +/// CleanAll), the daemon takes an atomic-swap branch: it calls +/// `[SNTConfigurator atomicallyApplyBundle:]`, which builds a fresh +/// in-memory dictionary from the bundle and performs a single in-memory +/// swap + single disk write. The mode-transition handler and CEL fallback +/// rule cache flush still fire after the swap; persistence for every other +/// slot is owned by the atomic-apply call. /// - (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply; diff --git a/Source/santad/SNTDaemonControlController.mm b/Source/santad/SNTDaemonControlController.mm index af97a2051..b17bc2db3 100644 --- a/Source/santad/SNTDaemonControlController.mm +++ b/Source/santad/SNTDaemonControlController.mm @@ -473,19 +473,34 @@ - (void)disableUnknownEventUpload:(void (^)(BOOL))reply { - (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply { SNTConfigurator* configurator = [SNTConfigurator configurator]; - // PostflightConfigBundle sets the bundle's syncType to Normal only when - // the in-flight sync was Clean or CleanAll (the directive is consumed by - // resetting it on disk). That exact transition is the signal that this - // bundle is a clean-sync postflight: wipe persisted sync state before - // applying so settings the server stops sending don't linger. The - // per-key setters that follow recreate `sync-state.plist` from the - // bundle's contents. - [result syncType:^(SNTSyncType val) { - if (val == SNTSyncTypeNormal) { - [configurator clearSyncState]; - } + __block BOOL atomicReplace = NO; + [result clearSyncStateBeforeApply:^(BOOL v) { + atomicReplace = v; }]; + if (atomicReplace) { + // Capture pre-swap CEL state so the post-swap comparison detects "rules + // disappeared from the server's config" as a change too — not just + // modifications. + NSData* oldCELData = [SNTCELFallbackRule serializeArray:[configurator celFallbackRules]]; + + [configurator atomicallyApplyBundle:result]; + + // Side-effects pass. Persistence is already done by atomicallyApplyBundle: + // for every slot except modeTransition. + [result modeTransition:^(SNTModeTransition* val) { + _temporaryMonitorMode->NewModeTransitionReceived(val); + }]; + + NSData* newCELData = [SNTCELFallbackRule serializeArray:[configurator celFallbackRules]]; + if (![oldCELData isEqualToData:newCELData] && self.flushCacheBlock) { + self.flushCacheBlock(FlushCacheMode::kAllCaches, FlushCacheReason::kCELFallbackRulesChanged); + } + + reply(); + return; + } + [result clientMode:^(SNTClientMode m) { [configurator setSyncServerClientMode:m]; }]; diff --git a/Source/santasyncservice/SNTSyncConfigBundle.mm b/Source/santasyncservice/SNTSyncConfigBundle.mm index 4e11ba688..fdbb2b073 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) { @@ -105,6 +106,10 @@ @interface SNTConfigBundle (ConfigBundleCreator) bundle.fullSyncLastSuccess = [NSDate now]; + if (syncState.syncType != SNTSyncTypeNormal) { + bundle.clearSyncStateBeforeApply = @(YES); + } + return bundle; } diff --git a/Source/santasyncservice/SNTSyncConfigBundleTest.mm b/Source/santasyncservice/SNTSyncConfigBundleTest.mm index f464c7220..bb8b1140e 100644 --- a/Source/santasyncservice/SNTSyncConfigBundleTest.mm +++ b/Source/santasyncservice/SNTSyncConfigBundleTest.mm @@ -315,4 +315,53 @@ - (void)testPostflightConfigBundleOmitsPushTokenChainWhenJWTsMissing { XCTAssertNil(bundle.pushTokenChain); } +- (void)testPostflightConfigBundleSetsClearSyncStateBeforeApplyOnCleanSync { + for (SNTSyncType type : (SNTSyncType[]){SNTSyncTypeClean, SNTSyncTypeCleanAll}) { + SNTSyncState* state = [[SNTSyncState alloc] init]; + state.syncType = type; + SNTConfigBundle* bundle = PostflightConfigBundle(state); + + __block BOOL fired = NO; + __block BOOL value = NO; + [bundle clearSyncStateBeforeApply:^(BOOL v) { + fired = YES; + value = v; + }]; + + XCTAssertTrue(fired, + @"PostflightConfigBundle must set clearSyncStateBeforeApply for syncType=%lu", + (unsigned long)type); + XCTAssertTrue(value); + } +} + +- (void)testPostflightConfigBundleOmitsClearSyncStateBeforeApplyOnNormalSync { + SNTSyncState* state = [[SNTSyncState alloc] init]; + state.syncType = SNTSyncTypeNormal; + SNTConfigBundle* bundle = PostflightConfigBundle(state); + + __block BOOL fired = NO; + [bundle clearSyncStateBeforeApply:^(BOOL v) { + fired = YES; + }]; + XCTAssertFalse(fired); +} + +- (void)testNonPostflightFactoriesNeverSetClearSyncStateBeforeApply { + SNTSyncState* state = [[SNTSyncState alloc] init]; + NSArray* bundles = @[ + PreflightConfigBundle(state), + RuleSyncConfigBundle(), + SyncTypeConfigBundle(SNTSyncTypeClean), + ]; + + for (SNTConfigBundle* bundle in bundles) { + __block BOOL fired = NO; + [bundle clearSyncStateBeforeApply:^(BOOL v) { + fired = YES; + }]; + XCTAssertFalse(fired); + } +} + @end diff --git a/Source/santasyncservice/SNTSyncManager.h b/Source/santasyncservice/SNTSyncManager.h index 1586f5b88..d259981c6 100644 --- a/Source/santasyncservice/SNTSyncManager.h +++ b/Source/santasyncservice/SNTSyncManager.h @@ -64,9 +64,9 @@ /// Pass `syncType` to control rule cleanup: `SNTSyncTypeNormal` for a regular /// sync, `SNTSyncTypeClean` to remove non-transitive rules, or /// `SNTSyncTypeCleanAll` to remove all rules. On Clean and CleanAll, the -/// postflight bundle signals the daemon to wipe persisted sync-managed -/// state before applying the bundle so settings the server stops sending -/// no longer linger. +/// postflight bundle sets `clearSyncStateBeforeApply` so the daemon +/// atomically replaces the persisted sync state instead of merging +/// per-key — settings the server stops sending no longer linger on disk. /// - (void)syncType:(SNTSyncType)syncType withReply:(void (^)(SNTSyncStatusType))reply; diff --git a/Source/santasyncservice/SNTSyncTest.mm b/Source/santasyncservice/SNTSyncTest.mm index bf9f21365..185630ecc 100644 --- a/Source/santasyncservice/SNTSyncTest.mm +++ b/Source/santasyncservice/SNTSyncTest.mm @@ -19,6 +19,7 @@ #import "Source/common/MOLXPCConnection.h" #import "Source/common/SNTCommonEnums.h" +#import "Source/common/SNTConfigBundle.h" #import "Source/common/SNTConfigurator.h" #import "Source/common/SNTModeTransition.h" #import "Source/common/SNTRule.h" @@ -1423,4 +1424,71 @@ - (void)testEventUploadRuleId { XCTAssertTrue([sut sync]); } +- (void)testCleanSyncPostflightBundleHasClearSyncStateBeforeApply { + // Drive a clean-sync flow through the two stages that ship + // updateSyncSettings: bundles bracketing the in-flight syncType (preflight + // and postflight), capture every bundle, and assert that exactly one of + // them (the postflight bundle) sets clearSyncStateBeforeApply == YES while + // none of the others do. This pins the contract that Task 4's clean-sync + // branch depends on: only the postflight bundle triggers the daemon's + // atomic swap; preflight bundles never carry the flag. + NSMutableArray* capturedBundles = [NSMutableArray array]; + OCMStub([self.daemonConnRop updateSyncSettings:[OCMArg any] reply:[OCMArg any]]) + .andDo(^(NSInvocation* inv) { + SNTConfigBundle* __unsafe_unretained bundle = nil; + [inv getArgument:&bundle atIndex:2]; + @synchronized(capturedBundles) { + [capturedBundles addObject:bundle]; + } + void (^__unsafe_unretained reply)(void) = nil; + [inv getArgument:&reply atIndex:3]; + if (reply) reply(); + }); + + // Default daemon stubs report Normal as the requested syncType; the server's + // preflight response below will override that to Clean. + [self setupDefaultDaemonConnResponses]; + + // Stage 1: preflight. Stub a server response that elects a Clean sync so + // syncState.syncType becomes Clean entering postflight. + NSData* preflightBody = + [@"{\"sync_type\": \"CLEAN\", \"client_mode\": \"MONITOR\", \"batch_size\": 100}" + dataUsingEncoding:NSUTF8StringEncoding]; + [self stubRequestBody:preflightBody + response:nil + error:nil + validateBlock:^BOOL(NSURLRequest* req) { + return [req.URL.absoluteString containsString:@"/preflight/"]; + }]; + XCTAssertTrue([[[SNTSyncPreflight alloc] initWithState:self.syncState] sync]); + XCTAssertEqual(self.syncState.syncType, SNTSyncTypeClean, + @"Preflight must elect Clean for the postflight bundle to set the flag"); + + // Stage 2: postflight. Stub a generic response and run. + [self stubRequestBody:nil + response:nil + error:nil + validateBlock:^BOOL(NSURLRequest* req) { + return [req.URL.absoluteString containsString:@"/postflight/"]; + }]; + XCTAssertTrue([[[SNTSyncPostflight alloc] initWithState:self.syncState] sync]); + + NSUInteger withFlag = 0; + for (SNTConfigBundle* b in capturedBundles) { + __block BOOL fired = NO; + [b clearSyncStateBeforeApply:^(BOOL v) { + if (v) fired = YES; + }]; + if (fired) withFlag++; + } + XCTAssertGreaterThanOrEqual(capturedBundles.count, (NSUInteger)2, + @"Expected the clean-sync flow to ship at least two bundles " + @"(preflight + postflight); got %lu", + (unsigned long)capturedBundles.count); + XCTAssertEqual(withFlag, (NSUInteger)1, + @"Expected exactly one bundle with clearSyncStateBeforeApply=YES " + @"during a --clean sync (the postflight bundle); got %lu", + (unsigned long)withFlag); +} + @end From ecb47cf952b74fdf4ac4556059996ef803891ad1 Mon Sep 17 00:00:00 2001 From: Sharvil Shah Date: Thu, 7 May 2026 19:28:54 +0530 Subject: [PATCH 4/6] feedback --- Source/santasyncservice/SNTSyncConfigBundleTest.mm | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Source/santasyncservice/SNTSyncConfigBundleTest.mm b/Source/santasyncservice/SNTSyncConfigBundleTest.mm index bb8b1140e..51caddfe5 100644 --- a/Source/santasyncservice/SNTSyncConfigBundleTest.mm +++ b/Source/santasyncservice/SNTSyncConfigBundleTest.mm @@ -258,11 +258,6 @@ - (void)testSyncTypeConfigBundle { XCTAssertNil(bundle.celFallbackRules); } -// PostflightConfigBundle invariants: these tests pin behavior the SNT-357 -// atomic-replace path depends on. If a future refactor breaks either invariant, -// `replaceSyncStateWithBundle:` would silently leave the on-disk plist in an -// inconsistent state after a clean sync. - - (void)testPostflightConfigBundleAlwaysSetsFullSyncLastSuccess { for (SNTSyncType type : (SNTSyncType[]){SNTSyncTypeNormal, SNTSyncTypeClean, SNTSyncTypeCleanAll}) { @@ -271,7 +266,7 @@ - (void)testPostflightConfigBundleAlwaysSetsFullSyncLastSuccess { SNTConfigBundle* bundle = PostflightConfigBundle(state); XCTAssertNotNil(bundle.fullSyncLastSuccess, @"PostflightConfigBundle must set fullSyncLastSuccess " - @"for syncType=%lu (load-bearing for SNT-357 atomic replace).", + @"for syncType=%lu", (unsigned long)type); } } From 579269cc6a90e00b6a74083a8559803b09bc846b Mon Sep 17 00:00:00 2001 From: Sharvil Shah Date: Thu, 7 May 2026 19:53:43 +0530 Subject: [PATCH 5/6] fixes --- Source/santad/SNTDaemonControlController.mm | 3 ++- Source/santasyncservice/SNTSyncConfigBundleTest.mm | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Source/santad/SNTDaemonControlController.mm b/Source/santad/SNTDaemonControlController.mm index b17bc2db3..9b6c8417a 100644 --- a/Source/santad/SNTDaemonControlController.mm +++ b/Source/santad/SNTDaemonControlController.mm @@ -493,7 +493,8 @@ - (void)updateSyncSettings:(SNTConfigBundle*)result reply:(void (^)(void))reply }]; NSData* newCELData = [SNTCELFallbackRule serializeArray:[configurator celFallbackRules]]; - if (![oldCELData isEqualToData:newCELData] && self.flushCacheBlock) { + if (oldCELData != newCELData && ![oldCELData isEqualToData:newCELData] && + self.flushCacheBlock) { self.flushCacheBlock(FlushCacheMode::kAllCaches, FlushCacheReason::kCELFallbackRulesChanged); } diff --git a/Source/santasyncservice/SNTSyncConfigBundleTest.mm b/Source/santasyncservice/SNTSyncConfigBundleTest.mm index 51caddfe5..d7fbbfda3 100644 --- a/Source/santasyncservice/SNTSyncConfigBundleTest.mm +++ b/Source/santasyncservice/SNTSyncConfigBundleTest.mm @@ -308,6 +308,12 @@ - (void)testPostflightConfigBundleOmitsPushTokenChainWhenJWTsMissing { syncState.pushIssuerJWT = @"issuerToken"; bundle = PostflightConfigBundle(syncState); XCTAssertNil(bundle.pushTokenChain); + + // Symmetric: only pushJWT set, pushIssuerJWT missing — still no chain. + syncState.pushIssuerJWT = nil; + syncState.pushJWT = @"userToken"; + bundle = PostflightConfigBundle(syncState); + XCTAssertNil(bundle.pushTokenChain); } - (void)testPostflightConfigBundleSetsClearSyncStateBeforeApplyOnCleanSync { From 4bf9d2e288c751adb87e56c5019422a0e4494d44 Mon Sep 17 00:00:00 2001 From: Sharvil Shah Date: Thu, 7 May 2026 20:01:30 +0530 Subject: [PATCH 6/6] fixes --- Source/santasyncservice/SNTSyncConfigBundle.mm | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Source/santasyncservice/SNTSyncConfigBundle.mm b/Source/santasyncservice/SNTSyncConfigBundle.mm index fdbb2b073..1e98fe9e1 100644 --- a/Source/santasyncservice/SNTSyncConfigBundle.mm +++ b/Source/santasyncservice/SNTSyncConfigBundle.mm @@ -69,10 +69,10 @@ @interface SNTConfigBundle (ConfigBundleCreator) SNTConfigBundle* bundle = [[SNTConfigBundle alloc] init]; bundle.clientMode = syncState.clientMode ? @(syncState.clientMode) : nil; - // Clean syncs reset SyncTypeRequired to Normal. The daemon recognizes - // this exact transition (`syncType == @(Normal)`) as the signal that - // this is a clean-sync postflight bundle and clears persisted sync - // state before applying. + // After a Clean or CleanAll sync, reset SyncTypeRequired to Normal so the + // directive doesn't repeat on the next sync. The daemon's atomic-swap + // branch is gated by `clearSyncStateBeforeApply` (set below), not by this + // syncType value. bundle.syncType = syncState.syncType != SNTSyncTypeNormal ? @(SNTSyncTypeNormal) : nil; bundle.allowlistRegex = syncState.allowlistRegex; bundle.blocklistRegex = syncState.blocklistRegex; @@ -106,6 +106,9 @@ @interface SNTConfigBundle (ConfigBundleCreator) bundle.fullSyncLastSuccess = [NSDate now]; + // On Clean/CleanAll syncs, signal the daemon to atomically replace the + // persisted sync state instead of merging per-key, so settings the server + // has stopped sending no longer linger on disk. if (syncState.syncType != SNTSyncTypeNormal) { bundle.clearSyncStateBeforeApply = @(YES); }