PR feedback

mlw · Nov 26, 2024 · 5b55659 · 5b55659
1 parent ee42f28
commit 5b55659
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 27 deletions.
diff --git a/Source/common/SNTConfigurator.m b/Source/common/SNTConfigurator.m
@@ -1045,12 +1045,22 @@ - (BOOL)enableForkAndExitLogging {
   return number ? [number boolValue] : NO;
 }
 
+// This method returns only the values that are of the expected string type.
+// The reasoning is that if a filter is attempted to be set, this method should
+// return some subset rather than `nil`. Since `nil` effectively means to log
+// everything, returning it would be akin to "failing open" even though some
+// filter configuration was attempted.
 - (NSArray<NSString *> *)telemetry {
-  NSArray *events = self.configState[kTelemetryKey];
+  NSArray *configuredEvents = self.configState[kTelemetryKey];
+  if (!configuredEvents) {
+    return nil;
+  }
 
-  for (id event in events) {
-    if (![event isKindOfClass:[NSString class]]) {
-      return nil;
+  NSMutableArray *events = [[NSMutableArray alloc] initWithCapacity:configuredEvents.count];
+
+  for (id event in configuredEvents) {
+    if ([event isKindOfClass:[NSString class]]) {
+      [events addObject:event];
     }
   }
 

diff --git a/Source/common/TelemetryEventMap.mm b/Source/common/TelemetryEventMap.mm
@@ -64,7 +64,7 @@ TelemetryEvent TelemetryConfigToBitmask(NSArray<NSString *> *telemetry,
       mask |= EventNameToMask(santa::NSStringToUTF8StringView([event_name lowercaseString]));
     }
   } else {
-    mask = EventNameToMask("everything");
+    mask = TelemetryEvent::kEverything;
 
     if (enableForkAndExitLogging == false) {
       mask &= (~TelemetryEvent::kFork & ~TelemetryEvent::kExit);

diff --git a/Source/santad/Santad.mm b/Source/santad/Santad.mm
@@ -400,31 +400,33 @@ void SantadMain(std::shared_ptr<EndpointSecurityAPI> esapi, std::shared_ptr<Logg
                                             FlushCacheReason::kEntitlementsPrefixFilterChanged);
               [authorizer_client clearCache];
             }],
-    [[SNTKVOManager alloc] initWithObject:configurator
-                                 selector:@selector(telemetry)
-                                     type:[NSArray class]
-                                 callback:^(NSArray *oldValue, NSArray *newValue) {
-                                   if (!oldValue && !newValue) {
-                                     return;
-                                   }
+    [[SNTKVOManager alloc]
+      initWithObject:configurator
+            selector:@selector(telemetry)
+                type:[NSArray class]
+            callback:^(NSArray *oldValue, NSArray *newValue) {
+              if (!oldValue && !newValue) {
+                return;
+              }
 
-                                   // Ensure the new array is composed of strings
-                                   for (id element in newValue) {
-                                     if (![element isKindOfClass:[NSString class]]) {
-                                       return;
-                                     }
-                                   }
+              // Ensure the new array is composed of strings
+              for (id element in newValue) {
+                if (![element isKindOfClass:[NSString class]]) {
+                  LOGW(@"Expected type in Telemetry config. Want String. Got: %@: value: %@",
+                       [element class], element);
+                  return;
+                }
+              }
 
-                                   if ([oldValue isEqualToArray:newValue]) {
-                                     return;
-                                   }
+              if ([oldValue isEqualToArray:newValue]) {
+                return;
+              }
 
-                                   LOGI(@"Telemetry changed: %@ -> %@",
-                                        [oldValue componentsJoinedByString:@","],
-                                        [newValue componentsJoinedByString:@","]);
-                                   logger->SetTelemetryMask(santa::TelemetryConfigToBitmask(
-                                     newValue, configurator.enableForkAndExitLogging));
-                                 }],
+              LOGI(@"Telemetry changed: %@ -> %@", [oldValue componentsJoinedByString:@","],
+                   [newValue componentsJoinedByString:@","]);
+              logger->SetTelemetryMask(
+                santa::TelemetryConfigToBitmask(newValue, configurator.enableForkAndExitLogging));
+            }],
     [[SNTKVOManager alloc] initWithObject:configurator
                                  selector:@selector(enableForkAndExitLogging)
                                      type:[NSNumber class]