Updates

dftimewolf/lib/collectors/grr_hosts.py

@@ -1240,8 +1240,12 @@ def _ProcessQuery(
       client: the GRR Client.
       osquery_container: the OSQuery.
     """
+    query = osquery_container.query
+    if not query.strip().endswith(';')
+      query += ';'
+
     flow_args = osquery_flows.OsqueryFlowArgs()
-    flow_args.query = osquery_container.query
+    flow_args.query = query
     flow_args.timeout_millis = self.timeout_millis
     flow_args.ignore_stderr_errors = self.ignore_stderr_errors
     flow_args.configuration_content = osquery_container.configuration_content
@@ -1260,7 +1264,6 @@ def _ProcessQuery(
 
     name = osquery_container.name
     description = osquery_container.description
-    query = osquery_container.query
     flow_identifier = flow_id
     client_identifier = client.client_id
 

dftimewolf/lib/collectors/grr_hunt.py

@@ -397,8 +397,12 @@ def Process(self) -> None:
     osquery_containers = self.GetContainers(containers.OsqueryQuery)
 
     for osquery_container in osquery_containers:
+      query = osquery_container.query
+      if not query.strip().endswith(';')
+        query += ';'
+
       hunt_args = osquery_flows.OsqueryFlowArgs()
-      hunt_args.query = osquery_container.query
+      hunt_args.query = query
       hunt_args.timeout_millis = self.timeout_millis
       hunt_args.ignore_stderr_errors = self.ignore_stderr_errors
       hunt_args.configuration_content = osquery_container.configuration_content

dftimewolf/lib/collectors/osquery.py

@@ -54,7 +54,9 @@ def _ValidateOsquery(self, query: str) -> bool:
       True if the query appears to be valid, False otherwise
     """
     # TODO(sydp): add more checks.
-    return query.upper().startswith('SELECT ')
+    return (
+        query.strip().upper().startswith('SELECT ') 
+        and query.strip().endswith(';'))
 
   def _ParsePlatforms(self, platforms: str) -> List[str]:
     """Parse and normalise the platforms value from an osquery pack.

tests/lib/collectors/grr_hosts.py

@@ -980,7 +980,7 @@ def testProcess(self, mock_LaunchFlow, mock_DownloadResults, _):
         mock_grr_hosts.MOCK_CLIENT_RECENT,
         'OsqueryFlow',
         osquery_pb2.OsqueryFlowArgs(
-            query='SELECT * FROM processes',
+            query='SELECT * FROM processes;',
             timeout_millis=300000,
             ignore_stderr_errors=False,
             configuration_content='',

tests/lib/collectors/grr_hunt.py

@@ -156,7 +156,7 @@ def testProcess(self):
     # extract call kwargs
     call_kwargs = self.mock_grr_api.CreateHunt.call_args[1]
     self.assertEqual(call_kwargs['flow_args'].query,
-                     'SELECT * FROM processes')
+                     'SELECT * FROM processes;')
     self.assertEqual(call_kwargs['flow_args'].timeout_millis,
                      300000)
     self.assertEqual(call_kwargs['flow_args'].ignore_stderr_errors, False)

tests/lib/collectors/osquery.py

@@ -44,6 +44,11 @@ def testSetupQueryError(self) -> None:
 
     self.assertEqual(context.exception.message, 'No valid osquery collected.')
 
+    with self.assertRaises(DFTimewolfError) as context:
+      self.osquery_collector.SetUp(query='SELECT * FROM processes', paths='')
+
+    self.assertEqual(context.exception.message, 'No valid osquery collected.')
+
   def testSetupPathsError(self) -> None:
     """Tests the collector's Setup() method with invalid paths parameter."""
     test_empty_data = ""
@@ -69,31 +74,31 @@ def testSetUpConfigurationError(self) -> None:
     """Tests the collector's SetUp() function with invalid configuration."""
     with self.assertRaises(DFTimewolfError) as context:
       self.osquery_collector.SetUp(
-          query='SELECT * FROM processes', paths='',
+          query='SELECT * FROM processes;', paths='',
           configuration_content='test', remote_configuration_path='test')
     self.assertEqual(
         context.exception.message,
         'Only one configuration argument can be set.')
 
     with self.assertRaises(DFTimewolfError) as context:
       self.osquery_collector.SetUp(
-          query='SELECT * FROM processes', paths='',
+          query='SELECT * FROM processes;', paths='',
           local_configuration_path ='test', remote_configuration_path='test')
     self.assertEqual(
         context.exception.message,
         'Only one configuration argument can be set.')
 
     with self.assertRaises(DFTimewolfError) as context:
       self.osquery_collector.SetUp(
-          query='SELECT * FROM processes', paths='',
+          query='SELECT * FROM processes;', paths='',
           local_configuration_path ='test', configuration_content='test')
     self.assertEqual(
         context.exception.message,
         'Only one configuration argument can be set.')
 
     with self.assertRaises(DFTimewolfError) as context:
       self.osquery_collector.SetUp(
-          query='SELECT * from processes', paths='',
+          query='SELECT * from processes;', paths='',
           configuration_content='invalid content')
     self.assertEqual(
         context.exception.message,
@@ -102,7 +107,7 @@ def testSetUpConfigurationError(self) -> None:
   def testSetUpRemoteConfigurationPath(self) -> None:
     """Tests the collector's SetUp() function with the remote config path."""
     self.osquery_collector.SetUp(
-        query='SELECT * from test',
+        query='SELECT * from test;',
         paths='ok',
         remote_configuration_path='/test/path')
     self.assertEqual(self.osquery_collector.configuration_path, '/test/path')
@@ -113,7 +118,7 @@ def testSetUpLocalConfigurationPath(self) -> None:
         'builtins.open',
         new=mock.mock_open(read_data='{"test": "test"}')) as _:
       self.osquery_collector.SetUp(
-          query='SELECT * from test',
+          query='SELECT * from test;',
           paths='ok',
           local_configuration_path='test')
     self.assertEqual(
@@ -122,7 +127,7 @@ def testSetUpLocalConfigurationPath(self) -> None:
   def testSetUpConfigurationContent(self) -> None:
     """Tests the collector's SetUp() function with configuration content."""
     self.osquery_collector.SetUp(
-        query='SELECT * from test',
+        query='SELECT * from test;',
         paths='ok',
         configuration_content='{"test": "test"}')
     self.assertEqual(
@@ -131,7 +136,7 @@ def testSetUpConfigurationContent(self) -> None:
   def testSetUpFileCollectionColumns(self) -> None:
     """Tests the collector's SetUp() function with file collection columns."""
     self.osquery_collector.SetUp(
-        query='SELECT * from test',
+        query='SELECT * from test;',
         paths='ok',
         file_collection_columns='a,b')
     self.assertEqual(
@@ -142,7 +147,7 @@ def testProcessTextFile(self, mock_exists) -> None:
     """Tests the collector's Process() function with a text file."""
     mock_exists.return_value = True
 
-    test_ok_data = "SELECT * FROM processes"
+    test_ok_data = "SELECT * FROM processes;"
 
     with mock.patch(
         'builtins.open',
@@ -153,7 +158,7 @@ def testProcessTextFile(self, mock_exists) -> None:
 
     containers = self.osquery_collector.GetContainers(OsqueryQuery)
     self.assertEqual(len(containers), 1)
-    self.assertEqual(containers[0].query, "SELECT * FROM processes")
+    self.assertEqual(containers[0].query, "SELECT * FROM processes;")
     self.assertEqual(containers[0].configuration_content, '')
     self.assertEqual(containers[0].configuration_path, '')