Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 156 additions & 0 deletions unit_tests/engine/test_rule_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 10 additions & 9 deletions userspace/engine/falco_load_result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(falco::load_result::warning_code::LOAD_COMPILE_CONDITION) +
1,
static_cast<int>(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) {
Expand All @@ -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<int>(falco::load_result::warning_code::LOAD_COMPILE_CONDITION) +
1,
static_cast<int>(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) {
Expand Down Expand Up @@ -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<int>(falco::load_result::warning_code::LOAD_COMPILE_CONDITION) +
1,
static_cast<int>(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) {
Expand Down
3 changes: 2 additions & 1 deletion userspace/engine/falco_load_result.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 19 additions & 1 deletion userspace/engine/rule_json_schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ const char rule_schema_string[] = LONG_STRING_CONST(
"type": "string"
}
},
"warn_evttypes": {
"type": "boolean"
},
"skip-if-unknown-filter": {
"type": "boolean"
}
Expand Down Expand Up @@ -153,7 +156,7 @@ const char rule_schema_string[] = LONG_STRING_CONST(
"append",
"replace"
],
"title": "Priority"
"title": "OverriddenItem"
},
"Override": {
"type": "object",
Expand All @@ -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,
Expand Down
58 changes: 58 additions & 0 deletions userspace/engine/rule_loader_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,23 @@ static void read_rule_exceptions(
exceptions = decoded;
}

static void warn_unknown_keys(const YAML::Node& item,
const std::set<std::string>& 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<std::string>();
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<std::string>& expected_keys,
const std::set<std::string>& overrides,
const std::string& override_type,
Expand Down Expand Up @@ -460,6 +477,10 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg,
}

collector.define(cfg, v);

static const std::set<std::string> 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,
Expand Down Expand Up @@ -514,6 +535,14 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg,

collector.define(cfg, v);
}

static const std::set<std::string> 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
Expand Down Expand Up @@ -559,6 +588,12 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg,
} else {
collector.define(cfg, v);
}

static const std::set<std::string> 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
Expand Down Expand Up @@ -610,6 +645,12 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg,
} else {
collector.define(cfg, v);
}

static const std::set<std::string> expected_macro_keys{"macro",
"condition",
"append",
"override"};
warn_unknown_keys(item, expected_macro_keys, cfg, ctx);
} else if(item["rule"].IsDefined()) {
std::string name;

Expand Down Expand Up @@ -898,6 +939,23 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg,
collector.define(cfg, v);
}
}

static const std::set<std::string> 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,
Expand Down
Loading