diff --git a/unit_tests/engine/test_rule_loader.cpp b/unit_tests/engine/test_rule_loader.cpp index d6a4001c5d8..6f3898695f3 100644 --- a/unit_tests/engine/test_rule_loader.cpp +++ b/unit_tests/engine/test_rule_loader.cpp @@ -1328,6 +1328,162 @@ TEST_F(test_falco_engine, empty_string_source_addl_rule) { EXPECT_TRUE(load_rules(rules_content, "rules.yaml")); } +// Phase 1: Schema correctness — no false positives for valid rule properties + +TEST_F(test_falco_engine, rule_with_warn_evttypes) { + std::string rules_content = R"END( +- rule: test_rule + desc: test rule description + condition: evt.type = close + output: user=%user.name command=%proc.cmdline file=%fd.name + priority: INFO + warn_evttypes: false +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")) << m_load_result_string; + ASSERT_VALIDATION_STATUS(yaml_helper::validation_ok) << m_load_result->schema_validation(); +} + +TEST_F(test_falco_engine, rule_with_skip_if_unknown_filter) { + std::string rules_content = R"END( +- rule: test_rule + desc: test rule description + condition: evt.type = close + output: user=%user.name command=%proc.cmdline file=%fd.name + priority: INFO + skip-if-unknown-filter: true +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")) << m_load_result_string; + ASSERT_VALIDATION_STATUS(yaml_helper::validation_ok) << m_load_result->schema_validation(); +} + +TEST_F(test_falco_engine, override_replace_warn_evttypes) { + std::string rules_content = R"END( +- rule: test_rule + desc: test rule description + condition: evt.type = close + output: user=%user.name command=%proc.cmdline file=%fd.name + priority: INFO + warn_evttypes: true + +- rule: test_rule + warn_evttypes: false + override: + warn_evttypes: replace +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")) << m_load_result_string; + ASSERT_VALIDATION_STATUS(yaml_helper::validation_ok) << m_load_result->schema_validation(); +} + +TEST_F(test_falco_engine, override_replace_capture) { + std::string rules_content = R"END( +- rule: test_rule + desc: test rule description + condition: evt.type = close + output: user=%user.name command=%proc.cmdline file=%fd.name + priority: INFO + +- rule: test_rule + capture: true + override: + capture: replace +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")) << m_load_result_string; + ASSERT_VALIDATION_STATUS(yaml_helper::validation_ok) << m_load_result->schema_validation(); +} + +TEST_F(test_falco_engine, override_replace_tags) { + std::string rules_content = R"END( +- rule: test_rule + desc: test rule description + condition: evt.type = close + output: user=%user.name command=%proc.cmdline file=%fd.name + priority: INFO + tags: [filesystem] + +- rule: test_rule + tags: [network] + override: + tags: replace +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")) << m_load_result_string; + ASSERT_VALIDATION_STATUS(yaml_helper::validation_ok) << m_load_result->schema_validation(); +} + +// Phase 2: Unknown key detection + +TEST_F(test_falco_engine, rule_unknown_key) { + std::string rules_content = R"END( +- rule: test_rule + desc: test rule description + condition: evt.type = close + output: user=%user.name command=%proc.cmdline file=%fd.name + priority: INFO + typo_field: some_value +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")) << m_load_result_string; + ASSERT_TRUE(check_warning_message("Unknown key 'typo_field'")); +} + +TEST_F(test_falco_engine, list_unknown_key) { + std::string rules_content = R"END( +- list: my_list + items: [cat, bash] + typo_field: some_value + +- rule: test_rule + desc: test rule description + condition: evt.type = close and proc.name in (my_list) + output: user=%user.name command=%proc.cmdline file=%fd.name + priority: INFO +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")) << m_load_result_string; + ASSERT_TRUE(check_warning_message("Unknown key 'typo_field'")); +} + +TEST_F(test_falco_engine, macro_unknown_key) { + std::string rules_content = R"END( +- macro: my_macro + condition: evt.type = close + typo_field: some_value + +- rule: test_rule + desc: test rule description + condition: my_macro + output: user=%user.name command=%proc.cmdline file=%fd.name + priority: INFO +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")) << m_load_result_string; + ASSERT_TRUE(check_warning_message("Unknown key 'typo_field'")); +} + +TEST_F(test_falco_engine, list_cross_type_key_priority) { + std::string rules_content = R"END( +- list: my_list + items: [cat, bash] + priority: INFO + +- rule: test_rule + desc: test rule description + condition: evt.type = close and proc.name in (my_list) + output: user=%user.name command=%proc.cmdline file=%fd.name + priority: INFO +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")) << m_load_result_string; + // The flat-union schema accepts 'priority' on a list (validation_ok), + // but procedural detection catches the cross-type misuse. + ASSERT_VALIDATION_STATUS(yaml_helper::validation_ok) << m_load_result->schema_validation(); + ASSERT_TRUE(check_warning_message("Unknown key 'priority'")); +} + TEST_F(test_falco_engine, deprecated_field_in_output) { std::string rules_content = R"END( - rule: test_rule_with_evt_dir_in_output diff --git a/userspace/engine/falco_load_result.cpp b/userspace/engine/falco_load_result.cpp index 496b28ecc6c..5066b0a76bb 100644 --- a/userspace/engine/falco_load_result.cpp +++ b/userspace/engine/falco_load_result.cpp @@ -73,12 +73,12 @@ static const std::string warning_codes[] = {"LOAD_UNKNOWN_SOURCE", "LOAD_EXCEPTION_NAME_NOT_UNIQUE", "LOAD_INVALID_MACRO_NAME", "LOAD_INVALID_LIST_NAME", - "LOAD_COMPILE_CONDITION"}; + "LOAD_COMPILE_CONDITION", + "LOAD_UNKNOWN_KEY"}; // Compile-time check to ensure warning_codes array has the correct size static_assert(std::size(warning_codes) == - static_cast(falco::load_result::warning_code::LOAD_COMPILE_CONDITION) + - 1, + static_cast(falco::load_result::warning_code::LOAD_UNKNOWN_KEY) + 1, "warning_codes array size must match the last warning_code enum value + 1"); const std::string& falco::load_result::warning_code_str(warning_code wc) { @@ -98,12 +98,12 @@ static const std::string warning_strings[] = {"Unknown event source", "Multiple exceptions defined with the same name", "Invalid macro name", "Invalid list name", - "Warning in rule condition"}; + "Warning in rule condition", + "Unknown key in item definition"}; // Compile-time check to ensure warning_strings array has the correct size static_assert(std::size(warning_strings) == - static_cast(falco::load_result::warning_code::LOAD_COMPILE_CONDITION) + - 1, + static_cast(falco::load_result::warning_code::LOAD_UNKNOWN_KEY) + 1, "warning_strings array size must match the last warning_code enum value + 1"); const std::string& falco::load_result::warning_str(warning_code wc) { @@ -131,12 +131,13 @@ static const std::string warning_descs[] = { "A rule is defining multiple exceptions with the same name", "A macro is defined with an invalid name", "A list is defined with an invalid name", - "A rule condition or output have been parsed with a warning"}; + "A rule condition or output have been parsed with a warning", + "An item in the rules content contains an unrecognized key. The key will be ignored. " + "This may indicate a typo or a property placed on the wrong item type."}; // Compile-time check to ensure warning_descs array has the correct size static_assert(std::size(warning_descs) == - static_cast(falco::load_result::warning_code::LOAD_COMPILE_CONDITION) + - 1, + static_cast(falco::load_result::warning_code::LOAD_UNKNOWN_KEY) + 1, "warning_descs array size must match the last warning_code enum value + 1"); const std::string& falco::load_result::warning_desc(warning_code wc) { diff --git a/userspace/engine/falco_load_result.h b/userspace/engine/falco_load_result.h index 733f44e8618..f77de457b2b 100644 --- a/userspace/engine/falco_load_result.h +++ b/userspace/engine/falco_load_result.h @@ -62,7 +62,8 @@ class load_result { LOAD_EXCEPTION_NAME_NOT_UNIQUE, LOAD_INVALID_MACRO_NAME, LOAD_INVALID_LIST_NAME, - LOAD_COMPILE_CONDITION + LOAD_COMPILE_CONDITION, + LOAD_UNKNOWN_KEY }; // The warning code as a string diff --git a/userspace/engine/rule_json_schema.h b/userspace/engine/rule_json_schema.h index 39c2a906d21..fbdcd9829c7 100644 --- a/userspace/engine/rule_json_schema.h +++ b/userspace/engine/rule_json_schema.h @@ -98,6 +98,9 @@ const char rule_schema_string[] = LONG_STRING_CONST( "type": "string" } }, + "warn_evttypes": { + "type": "boolean" + }, "skip-if-unknown-filter": { "type": "boolean" } @@ -153,7 +156,7 @@ const char rule_schema_string[] = LONG_STRING_CONST( "append", "replace" ], - "title": "Priority" + "title": "OverriddenItem" }, "Override": { "type": "object", @@ -179,6 +182,21 @@ const char rule_schema_string[] = LONG_STRING_CONST( }, "exceptions": { "$ref": "#/definitions/OverriddenItem" + }, + "capture": { + "$ref": "#/definitions/OverriddenItem" + }, + "capture_duration": { + "$ref": "#/definitions/OverriddenItem" + }, + "tags": { + "$ref": "#/definitions/OverriddenItem" + }, + "warn_evttypes": { + "$ref": "#/definitions/OverriddenItem" + }, + "skip-if-unknown-filter": { + "$ref": "#/definitions/OverriddenItem" } }, "minProperties": 1, diff --git a/userspace/engine/rule_loader_reader.cpp b/userspace/engine/rule_loader_reader.cpp index d5189b20e75..ad21c9001a4 100644 --- a/userspace/engine/rule_loader_reader.cpp +++ b/userspace/engine/rule_loader_reader.cpp @@ -404,6 +404,23 @@ static void read_rule_exceptions( exceptions = decoded; } +static void warn_unknown_keys(const YAML::Node& item, + const std::set& expected_keys, + rule_loader::configuration& cfg, + const rule_loader::context& ctx) { + if(!item.IsMap()) { + return; + } + for(auto it = item.begin(); it != item.end(); ++it) { + std::string key = it->first.as(); + if(expected_keys.find(key) == expected_keys.end()) { + cfg.res->add_warning(falco::load_result::warning_code::LOAD_UNKNOWN_KEY, + "Unknown key '" + key + "'. The key will be ignored.", + ctx); + } + } +} + inline static bool check_update_expected(std::set& expected_keys, const std::set& overrides, const std::string& override_type, @@ -460,6 +477,10 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg, } collector.define(cfg, v); + + static const std::set expected_required_engine_version_keys{ + "required_engine_version"}; + warn_unknown_keys(item, expected_required_engine_version_keys, cfg, ctx); } else if(item["required_plugin_versions"].IsDefined()) { const YAML::Node& req_plugin_vers = item["required_plugin_versions"]; rule_loader::context ctx(req_plugin_vers, @@ -514,6 +535,14 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg, collector.define(cfg, v); } + + static const std::set expected_required_plugin_versions_keys{ + "required_plugin_versions"}; + rule_loader::context rpv_ctx(item, + rule_loader::context::REQUIRED_PLUGIN_VERSIONS, + "", + parent); + warn_unknown_keys(item, expected_required_plugin_versions_keys, cfg, rpv_ctx); } else if(item["list"].IsDefined()) { std::string name; // Using tmp context until name is decoded @@ -559,6 +588,12 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg, } else { collector.define(cfg, v); } + + static const std::set expected_list_keys{"list", + "items", + "append", + "override"}; + warn_unknown_keys(item, expected_list_keys, cfg, ctx); } else if(item["macro"].IsDefined()) { std::string name; // Using tmp context until name is decoded @@ -610,6 +645,12 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg, } else { collector.define(cfg, v); } + + static const std::set expected_macro_keys{"macro", + "condition", + "append", + "override"}; + warn_unknown_keys(item, expected_macro_keys, cfg, ctx); } else if(item["rule"].IsDefined()) { std::string name; @@ -898,6 +939,23 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg, collector.define(cfg, v); } } + + static const std::set expected_rule_keys{"rule", + "condition", + "output", + "desc", + "priority", + "source", + "enabled", + "capture", + "capture_duration", + "warn_evttypes", + "skip-if-unknown-filter", + "tags", + "exceptions", + "override", + "append"}; + warn_unknown_keys(item, expected_rule_keys, cfg, ctx); } else { rule_loader::context ctx(item, rule_loader::context::RULES_CONTENT_ITEM, "", parent); cfg.res->add_warning(falco::load_result::warning_code::LOAD_UNKNOWN_ITEM,