parsers: Always apply the options parser first (#6050)

osquery/config/config.cpp

@@ -700,19 +700,9 @@ void Config::applyParsers(const std::string& source,
                           bool pack) {
   assert(obj.IsObject());
 
-  // Iterate each parser.
-  RecursiveLock lock(config_schedule_mutex_);
-  for (const auto& plugin : RegistryFactory::get().plugins("config_parser")) {
-    std::shared_ptr<ConfigParserPlugin> parser = nullptr;
-    try {
-      parser = std::dynamic_pointer_cast<ConfigParserPlugin>(plugin.second);
-    } catch (const std::bad_cast& /* e */) {
-      LOG(ERROR) << "Error casting config parser plugin: " << plugin.first;
-    }
-    if (parser == nullptr || parser.get() == nullptr) {
-      continue;
-    }
-
+  auto applyParser = [=](const std::shared_ptr<ConfigParserPlugin>& parser,
+                         const std::string& source,
+                         const rj::Value& obj) {
     // For each key requested by the parser, add a property tree reference.
     std::map<std::string, JSON> parser_config;
     for (const auto& key : parser->keys()) {
@@ -731,6 +721,39 @@ void Config::applyParsers(const std::string& source,
     // each top-level-config key. The parser may choose to update the config's
     // internal state
     parser->update(source, parser_config);
+  };
+
+  auto getParser = [=](const PluginRef& plugin, const std::string& name) {
+    std::shared_ptr<ConfigParserPlugin> parser = nullptr;
+    try {
+      parser = std::dynamic_pointer_cast<ConfigParserPlugin>(plugin);
+    } catch (const std::bad_cast& /* e */) {
+      LOG(ERROR) << "Error casting config parser plugin: " << name;
+    }
+    return parser;
+  };
+
+  RecursiveLock lock(config_schedule_mutex_);
+  auto plugins = RegistryFactory::get().plugins("config_parser");
+
+  // Always apply the options parser first, others may depend on flags/options.
+  auto options_plugin = plugins.find("options");
+  if (options_plugin != plugins.end()) {
+    auto parser = getParser(options_plugin->second, options_plugin->first);
+    if (parser != nullptr && parser.get() != nullptr) {
+      applyParser(parser, source, obj);
+    }
+  }
+
+  // Iterate each parser.
+  for (const auto& plugin : plugins) {
+    if (plugin.first == "options") {
+      continue;
+    }
+    auto parser = getParser(plugin.second, plugin.first);
+    if (parser != nullptr && parser.get() != nullptr) {
+      applyParser(parser, source, obj);
+    }
   }
 }
 

osquery/config/config.h

@@ -372,6 +372,7 @@ class Config : private boost::noncopyable {
   FRIEND_TEST(ConfigTests, test_get_scheduled_queries);
   FRIEND_TEST(ConfigTests, test_nonblacklist_query);
   FRIEND_TEST(OptionsConfigParserPluginTests, test_get_option);
+  FRIEND_TEST(OptionsConfigParserPluginTests, test_get_option_first);
   FRIEND_TEST(ViewsConfigParserPluginTests, test_add_view);
   FRIEND_TEST(ViewsConfigParserPluginTests, test_swap_view);
   FRIEND_TEST(ViewsConfigParserPluginTests, test_update_view);

plugins/config/parsers/decorators.cpp

@@ -182,28 +182,39 @@ void DecoratorsConfigParserPlugin::updateDecorations(const std::string& source,
 
   // Check if intervals are defined.
   auto& interval_key = kDecorationPointKeys.at(DECORATE_INTERVAL);
-  if (doc.doc().HasMember(interval_key)) {
-    const auto& interval = doc.doc()[interval_key];
-    if (interval.IsObject()) {
-      for (const auto& item : interval.GetObject()) {
-        auto rate = doc.valueToSize(item.name);
-        //      size_t rate = std::stoll(item.name.GetString());
-        if (rate % 60 != 0) {
-          LOG(WARNING) << "Invalid decorator interval rate " << rate
-                       << " in config source: " << source;
-          continue;
-        }
+  if (!doc.doc().HasMember(interval_key)) {
+    return;
+  }
 
-        // This is a valid interval, update the set of intervals to include
-        // this value. When intervals are checked this set is scanned, if a
-        // match is found, then the associated config data is executed.
-        if (item.value.IsArray()) {
-          for (const auto& interval_query : item.value.GetArray()) {
-            if (interval_query.IsString()) {
-              intervals_[source][rate].push_back(interval_query.GetString());
-            }
-          }
-        }
+  const auto& interval = doc.doc()[interval_key];
+  if (!interval.IsObject()) {
+    LOG(WARNING)
+        << "Invalid decorator interval configuration in config source: "
+        << source;
+    return;
+  }
+
+  for (const auto& item : interval.GetObject()) {
+    auto rate = doc.valueToSize(item.name);
+    //      size_t rate = std::stoll(item.name.GetString());
+    if (rate % 60 != 0) {
+      LOG(WARNING) << "Invalid decorator interval rate " << rate
+                   << " in config source: " << source;
+      continue;
+    }
+
+    if (!item.value.IsArray()) {
+      LOG(WARNING) << "Invalid decorator interval rate " << rate
+                   << " configuration in config source: " << source;
+      continue;
+    }
+
+    // This is a valid interval, update the set of intervals to include
+    // this value. When intervals are checked this set is scanned, if a
+    // match is found, then the associated config data is executed.
+    for (const auto& interval_query : item.value.GetArray()) {
+      if (interval_query.IsString()) {
+        intervals_[source][rate].push_back(interval_query.GetString());
       }
     }
   }

plugins/config/parsers/tests/options_tests.cpp

@@ -19,6 +19,8 @@ namespace osquery {
 
 DECLARE_bool(disable_database);
 
+FLAG(bool, test_options_race_parser, false, "");
+
 class OptionsConfigParserPluginTests : public testing::Test {
  protected:
   void SetUp() override {
@@ -32,6 +34,24 @@ class OptionsConfigParserPluginTests : public testing::Test {
   }
 };
 
+class TestOptionsRaceParser : public ConfigParserPlugin {
+ public:
+  std::vector<std::string> keys() const override {
+    return {"before_options"};
+  }
+
+  Status setUp() override {
+    return Status::success();
+  }
+
+  Status update(const std::string&, const ParserConfig&) override {
+    if (!FLAGS_test_options_race_parser) {
+      throw std::runtime_error("The flag test_options_race_parser is false");
+    }
+    return Status::success();
+  }
+};
+
 TEST_F(OptionsConfigParserPluginTests, test_get_option) {
   Config c;
   auto s = c.update(getTestConfigMap("test_parse_items.conf"));
@@ -44,6 +64,20 @@ TEST_F(OptionsConfigParserPluginTests, test_get_option) {
   c.reset();
 }
 
+TEST_F(OptionsConfigParserPluginTests, test_get_option_first) {
+  auto& rf = RegistryFactory::get();
+  auto options_race_parser = std::make_shared<TestOptionsRaceParser>();
+  rf.registry("config_parser")->add("before_options", options_race_parser);
+
+  Config c;
+  std::map<std::string, std::string> update;
+  update["options"] = "{\"options\": {\"test_options_race_parser\": true}}";
+  EXPECT_NO_THROW(c.update(update));
+
+  rf.registry("config_parser")->remove("before_options");
+  c.reset();
+}
+
 TEST_F(OptionsConfigParserPluginTests, test_unknown_option) {
   Config c;
   std::map<std::string, std::string> update;