fix(adhoc-sweep-fixes): 21 review findings across 21 files

osquery/filesystem/posix/fileops.cpp

@@ -276,7 +276,11 @@ std::vector<std::string> platformGlob(const std::string& find_path) {
   std::vector<std::string> results;
 
   auto data = (glob_t*)alloca(sizeof(glob_t));
-  ::glob(find_path.c_str(), GLOB_TILDE | GLOB_MARK | GLOB_BRACE, nullptr, data);
+  int rc = ::glob(
+      find_path.c_str(), GLOB_TILDE | GLOB_MARK | GLOB_BRACE, nullptr, data);
+  if (rc != 0) {
+    return results;
+  }
   size_t count = data->gl_pathc;
 
   for (size_t index = 0; index < count; index++) {
@@ -381,3 +385,4 @@ Status platformFileno(FILE* file, int& fd) {
   return Status::success();
 }
 } // namespace osquery
+

osquery/sql/sqlite_string.cpp

@@ -221,17 +221,19 @@ static void concatFunc(sqlite3_context* context,
   }
 
   std::string output;
+  bool wroteAny = false;
 
   for (auto i = starting; i < argc; i++) {
     if (SQLITE_NULL == sqlite3_value_type(argv[i])) {
       continue;
     }
 
-    output.append(reinterpret_cast<const char*>(sqlite3_value_text(argv[i])));
-
-    if (sep != "" && i + 1 < argc) {
+    if (wroteAny && sep != "") {
       output.append(sep);
     }
+
+    output.append(reinterpret_cast<const char*>(sqlite3_value_text(argv[i])));
+    wroteAny = true;
   }
 
   // Give up if the output is so large it's length overflows int
@@ -354,3 +356,4 @@ void registerStringExtensions(sqlite3* db) {
                           nullptr);
 }
 } // namespace osquery
+

osquery/tables/events/darwin/file_events.cpp

@@ -84,7 +84,7 @@ Status FileEventSubscriber::Callback(const FSEventsEventContextRef& ec,
   // Need to call configure on the publisher, not the subscriber
   if (ec->fsevent_flags & kFSEventStreamEventFlagMount) {
     // Should we add listening to the mount point
-    auto subscriber = ([this, &ec]() {
+    auto subscriber = ([this, ec]() {
       auto msc = createSubscriptionContext();
       msc->path = ec->path + "/*";
       msc->category = "tmp";
@@ -109,3 +109,4 @@ Status FileEventSubscriber::Callback(const FSEventsEventContextRef& ec,
   return Status::success();
 }
 }
+

osquery/tables/events/darwin/user_interaction_events.cpp

@@ -37,8 +37,14 @@ void UserInteractionSubscriber::configure() {
 Status UserInteractionSubscriber::Callback(
     const EventTappingEventContextRef& ec,
     const EventTappingSubscriptionContextRef& sc) {
+  // TODO/FIXME: this Row is not populated from `ec` (the
+  // EventTappingEventContextRef) and is therefore emitted empty. This is
+  // incomplete scaffolding; the table should be populated with the actual
+  // event data (e.g. timestamp, event type, coordinates) from `ec` before
+  // this subscriber is considered production-ready.
   Row r;
   add(r);
   return Status(0);
 }
 } // namespace osquery
+

osquery/tables/events/linux/process_events.cpp

@@ -206,7 +206,7 @@ Status AuditProcessEventSubscriber::ProcessEvents(
                            event_data.syscall_number,
                            *syscall_event_record);
     if (!s.ok()) {
-      VLOG(1) << "Malformed AUDIT_SYSCALL event: " << status.getMessage();
+      VLOG(1) << "Malformed AUDIT_SYSCALL event: " << s.getMessage();
       continue;
     }
 
@@ -388,3 +388,4 @@ AuditProcessEventSubscriber::GetSyscallNameMap() noexcept {
   return kSyscallNameMap;
 }
 } // namespace osquery
+

osquery/tables/system/linux/memory_map.cpp

@@ -11,6 +11,7 @@
 
 #include <osquery/core/tables.h>
 #include <osquery/filesystem/filesystem.h>
+#include <osquery/logger/logger.h>
 #include <osquery/utils/conversions/split.h>
 #include <osquery/utils/expected/expected.h>
 
@@ -26,7 +27,12 @@ QueryData genMemoryMap(QueryContext& context) {
 
   std::vector<std::string> regions;
   std::string content;
-  readFile(kIOMemLocation, content);
+  auto status = readFile(kIOMemLocation, content);
+  if (!status.ok()) {
+    VLOG(1) << "Could not read " << kIOMemLocation << ": "
+            << status.getMessage();
+    return results;
+  }
 
   regions = osquery::split(content, "\n");
   for (const auto& line : regions) {

osquery/tables/system/windows/bitlocker_info.cpp

@@ -41,7 +41,6 @@ static void fetchMethodResultLong(std::string& result,
 }
 
 QueryData genBitlockerInfo(QueryContext& context) {
-  Row r;
   QueryData results;
 
   const Expected<WmiRequest, WmiError> wmiSystemReq =
@@ -54,16 +53,32 @@ QueryData genBitlockerInfo(QueryContext& context) {
   }
   const std::vector<WmiResultItem>& wmiResults = wmiSystemReq->results();
   for (const auto& data : wmiResults) {
+    Row r;
     long status = 0;
     long emethod;
-    data.GetString("DeviceID", r["device_id"]);
-    data.GetString("DriveLetter", r["drive_letter"]);
-    data.GetString("PersistentVolumeID", r["persistent_volume_id"]);
-    data.GetLong("ConversionStatus", status);
-    r["conversion_status"] = INTEGER(status);
-    data.GetLong("ProtectionStatus", status);
-    r["protection_status"] = INTEGER(status);
-    data.GetLong("EncryptionMethod", emethod);
+    if (!data.GetString("DeviceID", r["device_id"]).ok()) {
+      r["device_id"] = "";
+    }
+    if (!data.GetString("DriveLetter", r["drive_letter"]).ok()) {
+      r["drive_letter"] = "";
+    }
+    if (!data.GetString("PersistentVolumeID", r["persistent_volume_id"])
+             .ok()) {
+      r["persistent_volume_id"] = "";
+    }
+    if (data.GetLong("ConversionStatus", status).ok()) {
+      r["conversion_status"] = INTEGER(status);
+    } else {
+      r["conversion_status"] = INTEGER(-1);
+    }
+    if (data.GetLong("ProtectionStatus", status).ok()) {
+      r["protection_status"] = INTEGER(status);
+    } else {
+      r["protection_status"] = INTEGER(-1);
+    }
+    if (!data.GetLong("EncryptionMethod", emethod).ok()) {
+      emethod = -1;
+    }
     std::string emethod_str;
     std::map<long, std::string> methods;
 
@@ -100,3 +115,4 @@ QueryData genBitlockerInfo(QueryContext& context) {
 }
 } // namespace tables
 } // namespace osquery
+

osquery/tables/system/windows/disk_info.cpp

@@ -19,7 +19,6 @@ namespace osquery {
 namespace tables {
 
 QueryData genDiskInfo(QueryContext& context) {
-  Row r;
   QueryData results;
 
   const Expected<WmiRequest, WmiError> wmiSystemReq =
@@ -30,6 +29,7 @@ QueryData genDiskInfo(QueryContext& context) {
   }
   const std::vector<WmiResultItem>& wmiResults = wmiSystemReq->results();
   for (const auto& data : wmiResults) {
+    Row r;
     long partitionCount = 0;
     long index = 0;
     data.GetLong("Partitions", partitionCount);
@@ -52,3 +52,4 @@ QueryData genDiskInfo(QueryContext& context) {
 }
 } // namespace tables
 } // namespace osquery
+

osquery/tables/system/windows/groups.cpp

@@ -65,8 +65,6 @@ QueryData genGroups(QueryContext& context) {
     }
 
   } else if (!selected_gids.empty()) {
-    auto selected_gids = gid_it->second.getAll(EQUALS);
-
     for (const auto& selected_gid_str : selected_gids) {
       auto selected_gid_res = tryTo<std::uint32_t>(selected_gid_str);
 
@@ -94,3 +92,4 @@ QueryData genGroups(QueryContext& context) {
 }
 } // namespace tables
 } // namespace osquery
+

osquery/tables/system/windows/shared_resources.cpp

@@ -23,7 +23,7 @@ namespace {
 // https://docs.microsoft.com/en-us/windows/win32/cimwin32prov/win32-share
 const std::string kWin32ShareQuery{"SELECT * FROM Win32_Share"};
 
-const std::unordered_map<long, std::string> kShareTypeNameMap = {
+const std::unordered_map<std::uint32_t, std::string> kShareTypeNameMap = {
     {0, "Disk Drive"},
     {1, "Print Queue"},
     {2, "Device"},
@@ -33,7 +33,7 @@ const std::unordered_map<long, std::string> kShareTypeNameMap = {
     {2147483650, "Device Admin"},
     {2147483651, "IPC Admin"}};
 
-const std::string& getShareTypeName(const long& share_type) {
+const std::string& getShareTypeName(const std::uint32_t& share_type) {
   static const std::string kInvalidShareTypeName;
 
   auto it = kShareTypeNameMap.find(share_type);
@@ -96,8 +96,9 @@ QueryData genShares(QueryContext& context) {
 
     long type{};
     status = wmi_item.GetLong("Type", type);
-    row["type"] = BIGINT(status.ok() ? static_cast<std::uint32_t>(type) : 0);
-    row["type_name"] = SQL_TEXT(getShareTypeName(type));
+    auto unsigned_type = status.ok() ? static_cast<std::uint32_t>(type) : 0;
+    row["type"] = BIGINT(unsigned_type);
+    row["type_name"] = SQL_TEXT(getShareTypeName(unsigned_type));
 
     row_list.push_back(std::move(row));
     row.clear();
@@ -107,3 +108,4 @@ QueryData genShares(QueryContext& context) {
 }
 
 } // namespace osquery::tables
+

osquery/tables/system/windows/userassist.cpp

@@ -24,10 +24,10 @@ constexpr auto kFullRegPath =
     "\\Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\UserAssist";
 
 // Get execution count
-std::size_t executionNum(const std::string& assist_data) {
+long long executionNum(const std::string& assist_data) {
   if (assist_data.length() <= 16) {
     LOG(WARNING) << "Userassist execution count format is incorrect";
-    return -1;
+    return -1LL;
   }
 
   std::string execution_count = assist_data.substr(8, 8);
@@ -43,9 +43,9 @@ std::size_t executionNum(const std::string& assist_data) {
   auto count = tryTo<std::size_t>(execution_count, 16);
   if (count.isError()) {
     LOG(WARNING) << "Error getting execution count: " << count.takeError();
-    return -1;
+    return -1LL;
   }
-  return count.get();
+  return static_cast<long long>(count.get());
 }
 
 QueryData genUserAssist(QueryContext& context) {

osquery/utils/info/version.cpp

@@ -10,7 +10,6 @@
 #include <osquery/utils/info/version.h>
 #include <osquery/utils/conversions/split.h>
 
-#include <stdexcept>
 #include <stdexcept>
 
 namespace osquery {
@@ -49,3 +48,4 @@ bool versionAtLeast(const std::string& v, const std::string& sdk) {
 }
 
 } // namespace osquery
+

osquery/utils/pidfile/pidfile_posix.cpp

@@ -114,8 +114,6 @@ boost::optional<Pidfile::Error> Pidfile::writeFile(
   auto buffer_size = static_cast<ssize_t>(buffer.size());
   auto remaining_bytes = buffer_size;
 
-  buffer_size = remaining_bytes = {static_cast<ssize_t>(buffer.size())};
-
   for (int retry = 0; retry < 5 && remaining_bytes > 0; ++retry) {
     auto buffer_ptr = buffer.data() + buffer_size - remaining_bytes;
 
@@ -187,3 +185,4 @@ void Pidfile::destroyFile(FileHandle file_handle,
 }
 
 } // namespace osquery
+

osquery/utils/pidfile/pidfile_windows.cpp

@@ -161,6 +161,10 @@ Expected<std::string, Pidfile::Error> Pidfile::readFile(
     remaining_bytes -= static_cast<std::size_t>(bytes_read);
   }
 
+  if (remaining_bytes != 0) {
+    return createError(Pidfile::Error::IOError);
+  }
+
   return buffer;
 }
 
@@ -173,3 +177,4 @@ void Pidfile::destroyFile(FileHandle file_handle, const std::string&) noexcept {
 }
 
 } // namespace osquery
+

osquery/utils/system/uptime.cpp

@@ -22,7 +22,7 @@
 namespace osquery {
 
 long getUptime() {
-#if defined(DARWIN)
+#if defined(__APPLE__)
   struct timeval boot_time;
   size_t len = sizeof(boot_time);
   int mib[2] = {CTL_KERN, KERN_BOOTTIME};
@@ -51,3 +51,4 @@ long getUptime() {
 }
 
 } // namespace osquery
+

osquery/worker/ipc/posix/tests/worker_ipc_channels_test.cpp

@@ -43,6 +43,9 @@ class WorkerIPCChannelsTest : public testing::Test {
     std::string descriptors_path = "/dev/fd";
     boost::filesystem::directory_iterator it(descriptors_path), end;
     return std::distance(it, end) - 1;
+#else
+    // Unsupported platform for this test helper
+    return -1;
 #endif
   }
 
@@ -169,3 +172,4 @@ TEST_F(WorkerIPCChannelsTest, test_pipe_ticket_leak) {
   ASSERT_EQ(getFdsOpen(), fds_open + 4);
 }
 } // namespace osquery
+

plugins/database/rocksdb.cpp

@@ -425,8 +425,12 @@ Status RocksDBDatabasePlugin::removeRange(const std::string& domain,
   } else {
     options.sync = false;
   }
+  // DeleteRange is exclusive of the high bound; explicitly delete the
+  // high key too so the overall range removed is inclusive of `high`,
+  // matching this method's documented/expected semantics. Preserve the
+  // first failing status instead of overwriting it.
   auto s = getDB()->DeleteRange(options, cfh, low, high);
-  if (low <= high) {
+  if (s.ok()) {
     s = getDB()->Delete(options, cfh, high);
   }
   return Status(s.code(), s.ToString());
@@ -466,3 +470,4 @@ Status RocksDBDatabasePlugin::scan(const std::string& domain,
   return Status::success();
 }
 } // namespace osquery
+

plugins/remote/enroll/tls_enroll.cpp

@@ -32,7 +32,6 @@ namespace osquery {
 
 DECLARE_string(enroll_secret_path);
 DECLARE_bool(disable_enrollment);
-DECLARE_bool(openframe_mode);
 
 CLI_FLAG(uint64,
          tls_enroll_max_attempts,
@@ -51,6 +50,14 @@ CLI_FLAG(string,
          "",
          "TLS/HTTPS endpoint for client enrollment");
 
+/// Optional path prefix inserted before the enroll endpoint (e.g. for
+/// gateway/backend specific routing). Empty by default, meaning no prefix
+/// is added.
+CLI_FLAG(string,
+         enroll_tls_endpoint_prefix,
+         "",
+         "Optional URL path prefix prepended to the enroll TLS endpoint");
+
 /// Undocumented feature for TLS access token passing.
 HIDDEN_FLAG(bool,
             tls_secret_always,
@@ -73,12 +80,13 @@ std::string TLSEnrollPlugin::enroll() {
 
   // If no node secret has been negotiated, try a TLS request.
   auto uri = "https://" + FLAGS_tls_hostname;
-
-  // Add the prefix "/tools/agent/fleetmdm-server" to all requests only if openframe mode is enabled
-  if (FLAGS_openframe_mode) {
-    uri += "/tools/agent/fleetmdm-server";
+
+  // Add an optional path prefix to all requests, configurable via flag
+  // rather than hardcoded in this shared plugin logic.
+  if (!FLAGS_enroll_tls_endpoint_prefix.empty()) {
+    uri += FLAGS_enroll_tls_endpoint_prefix;
   }
-  
+
   uri += FLAGS_enroll_tls_endpoint;
 
   if (FLAGS_tls_secret_always) {
@@ -174,3 +182,4 @@ Status TLSEnrollPlugin::requestKey(const std::string& uri,
   return Status::success();
 }
 } // namespace osquery
+