feat(engine): emit warning when a condition uses deprecated "evt.dir"

unit_tests/engine/test_filter_warning_resolver.cpp

@@ -15,14 +15,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+#include <string>
 #include <gtest/gtest.h>
 #include <engine/filter_warning_resolver.h>
 
 static bool warns(const std::string& condition) {
-	std::set<falco::load_result::warning_code> w;
 	auto ast = libsinsp::filter::parser(condition).parse();
-	filter_warning_resolver().run(ast.get(), w);
-	return !w.empty();
+	rule_loader::context ctx("test");
+	rule_loader::result res("test");
+	filter_warning_resolver().run(ctx, res, *ast.get());
+	return res.has_warnings();
 }
 
 TEST(WarningResolver, warnings_in_filtering_conditions) {
@@ -38,4 +40,8 @@ TEST(WarningResolver, warnings_in_filtering_conditions) {
 	ASSERT_TRUE(warns("ka.field intersects (otherval, <NA>)"));
 	ASSERT_TRUE(warns("ka.field pmatch (<NA>)"));
 	ASSERT_TRUE(warns("ka.field pmatch (otherval, <NA>)"));
+	ASSERT_TRUE(warns("evt.dir = <"));
+	ASSERT_TRUE(warns("evt.dir = >"));
+	ASSERT_TRUE(warns("proc.name=test and evt.dir = <"));
+	ASSERT_TRUE(warns("evt.dir = < and proc.name=test"));
 }

userspace/engine/falco_load_result.cpp

@@ -75,8 +75,14 @@ static const std::string warning_codes[] = {"LOAD_UNKNOWN_SOURCE",
                                             "LOAD_INVALID_LIST_NAME",
                                             "LOAD_COMPILE_CONDITION"};
 
+// 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,
+              "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) {
-	return warning_codes[wc];
+	return warning_codes[static_cast<int>(wc)];
 }
 
 static const std::string warning_strings[] = {"Unknown event source",
@@ -94,8 +100,14 @@ static const std::string warning_strings[] = {"Unknown event source",
                                               "Invalid list name",
                                               "Warning in rule condition"};
 
+// 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,
+              "warning_strings array size must match the last warning_code enum value + 1");
+
 const std::string& falco::load_result::warning_str(warning_code wc) {
-	return warning_strings[wc];
+	return warning_strings[static_cast<int>(wc)];
 }
 
 static const std::string warning_descs[] = {
@@ -121,6 +133,46 @@ static const std::string warning_descs[] = {
         "A list is defined with an invalid name",
         "A rule condition or output have been parsed with a warning"};
 
+// 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,
+              "warning_descs array size must match the last warning_code enum value + 1");
+
 const std::string& falco::load_result::warning_desc(warning_code wc) {
-	return warning_descs[wc];
+	return warning_descs[static_cast<int>(wc)];
+}
+
+static const std::string deprecated_fields[] = {"evt.dir"};
+
+// Compile-time check to ensure deprecated_fields array has the correct size
+static_assert(
+        std::size(deprecated_fields) ==
+                static_cast<int>(falco::load_result::deprecated_field::DEPRECATED_FIELD_NOT_FOUND),
+        "deprecated_fields array size must match DEPRECATED_FIELD_NOT_FOUND enum value");
+
+const std::string& falco::load_result::deprecated_field_str(deprecated_field df) {
+	return deprecated_fields[static_cast<int>(df)];
+}
+
+static const std::string deprecated_field_descs[] = {
+        "due to the drop of enter events, 'evt.dir = <' always evaluates to true, and 'evt.dir = "
+        ">' always evaluates to false. The rule expression can be simplified by removing the "
+        "condition on 'evt.dir'"};
+
+// Compile-time check to ensure deprecated_field_descs array has the correct size
+static_assert(
+        std::size(deprecated_field_descs) ==
+                static_cast<int>(falco::load_result::deprecated_field::DEPRECATED_FIELD_NOT_FOUND),
+        "deprecated_field_descs array size must match DEPRECATED_FIELD_NOT_FOUND enum value");
+
+const std::string& falco::load_result::deprecated_field_desc(deprecated_field df) {
+	return deprecated_field_descs[static_cast<int>(df)];
+}
+
+falco::load_result::deprecated_field falco::load_result::deprecated_field_from_str(
+        const std::string& f) {
+	return falco::load_result::deprecated_field(
+	        std::find(std::begin(deprecated_fields), std::end(deprecated_fields), f) -
+	        std::begin(deprecated_fields));
 }

userspace/engine/falco_load_result.h

@@ -46,7 +46,9 @@ class load_result {
 	// impact.
 	static const std::string& error_desc(error_code ec);
 
-	enum warning_code {
+	virtual ~load_result() = default;
+
+	enum class warning_code {
 		LOAD_UNKNOWN_SOURCE = 0,
 		LOAD_UNSAFE_NA_CHECK,
 		LOAD_NO_EVTTYPE,
@@ -63,8 +65,6 @@ class load_result {
 		LOAD_COMPILE_CONDITION
 	};
 
-	virtual ~load_result() = default;
-
 	// The warning code as a string
 	static const std::string& warning_code_str(warning_code ec);
 
@@ -75,6 +75,19 @@ class load_result {
 	// impact.
 	static const std::string& warning_desc(warning_code ec);
 
+	enum class deprecated_field { DEPRECATED_FIELD_EVT_DIR, DEPRECATED_FIELD_NOT_FOUND };
+
+	// The deprecated field as a string
+	static const std::string& deprecated_field_str(deprecated_field df);
+
+	// A longer description of what the deprecated field represents and the
+	// impact.
+	static const std::string& deprecated_field_desc(deprecated_field df);
+
+	// Return the deprecated field from a field string name, or DEPRECATED_FIELD_NOT_FOUND if the
+	// field is not deprecated
+	static deprecated_field deprecated_field_from_str(const std::string& f);
+
 	// If true, the rules were loaded successfully and can be used
 	// against events. If false, there were one or more
 	// errors--use one of the as_xxx methods to return information

userspace/engine/filter_warning_resolver.cpp

@@ -15,6 +15,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+#include <string>
 #include <libsinsp/sinsp.h>
 #include "filter_warning_resolver.h"
 
@@ -32,14 +33,30 @@ static inline bool is_equality_operator(const std::string& op) {
 	       op == "pmatch";
 }
 
-bool filter_warning_resolver::run(libsinsp::filter::ast::expr* filter,
-                                  std::set<load_result::warning_code>& warnings) const {
-	visitor v;
-	auto size = warnings.size();
+bool filter_warning_resolver::run(const rule_loader::context& ctx,
+                                  rule_loader::result& res,
+                                  libsinsp::filter::ast::expr& filter) const {
+	std::set<falco::load_result::warning_code> warnings;
+	std::set<falco::load_result::deprecated_field> deprecated_fields;
+	visitor v(warnings, deprecated_fields);
 	v.m_is_equality_check = false;
-	v.m_warnings = &warnings;
-	filter->accept(&v);
-	return warnings.size() > size;
+	filter.accept(&v);
+	for(auto& w : warnings) {
+		switch(w) {
+		case falco::load_result::warning_code::LOAD_DEPRECATED_ITEM:
+			// add a warning for each deprecated field
+			for(auto& deprecated_field : deprecated_fields) {
+				res.add_deprecated_field_warning(
+				        deprecated_field,
+				        falco::load_result::deprecated_field_desc(deprecated_field),
+				        ctx);
+			}
+			break;
+		default:
+			res.add_warning(w, "", ctx);
+		}
+	}
+	return !warnings.empty();
 }
 
 void filter_warning_resolver::visitor::visit(libsinsp::filter::ast::binary_check_expr* e) {
@@ -54,17 +71,24 @@ void filter_warning_resolver::visitor::visit(libsinsp::filter::ast::binary_check
 
 void filter_warning_resolver::visitor::visit(libsinsp::filter::ast::field_expr* e) {
 	m_last_node_is_unsafe_field = is_unsafe_field(e->field);
+
+	// Check for deprecated dir field usage
+	if(auto df = falco::load_result::deprecated_field_from_str(e->field);
+	   df != falco::load_result::deprecated_field::DEPRECATED_FIELD_NOT_FOUND) {
+		m_deprecated_fields->insert(df);
+		m_warnings->insert(falco::load_result::warning_code::LOAD_DEPRECATED_ITEM);
+	}
 }
 
 void filter_warning_resolver::visitor::visit(libsinsp::filter::ast::value_expr* e) {
 	if(m_is_equality_check && e->value == no_value) {
-		m_warnings->insert(load_result::LOAD_UNSAFE_NA_CHECK);
+		m_warnings->insert(falco::load_result::warning_code::LOAD_UNSAFE_NA_CHECK);
 	}
 }
 
 void filter_warning_resolver::visitor::visit(libsinsp::filter::ast::list_expr* e) {
 	if(m_is_equality_check &&
 	   std::find(e->values.begin(), e->values.end(), no_value) != e->values.end()) {
-		m_warnings->insert(load_result::LOAD_UNSAFE_NA_CHECK);
+		m_warnings->insert(falco::load_result::warning_code::LOAD_UNSAFE_NA_CHECK);
 	}
 }

userspace/engine/filter_warning_resolver.h

@@ -23,6 +23,7 @@ limitations under the License.
 #include <memory>
 #include "falco_common.h"
 #include "falco_load_result.h"
+#include "rule_loader.h"
 
 /*!
     \brief Searches for bad practices in filter conditions and
@@ -31,25 +32,23 @@ limitations under the License.
 class filter_warning_resolver {
 public:
 	/*!
-	    \brief Visits a filter AST and substitutes macro references
-	    according with all the definitions added through set_macro(),
-	    by replacing the reference with a clone of the macro AST.
+	    \brief Runs the filter warning resolver on a filter AST and adds the warnings to the result
+	   object \param ctx The context of the warning \param res The result to add the warnings to
 	    \param filter The filter AST to be visited
-	    \param warnings Set of strings to be filled with warning codes. This
-	    is not cleared up before the visit
-	    \param blocking Filled-out with true if at least one warning is
-	    found and at least one warning prevents the filter from being loaded
 	    \return true if at least one warning is generated
 	*/
-	bool run(libsinsp::filter::ast::expr* filter,
-	         std::set<falco::load_result::warning_code>& warnings) const;
+	bool run(const rule_loader::context& ctx,
+	         rule_loader::result& res,
+	         libsinsp::filter::ast::expr& filter) const;
 
 private:
 	struct visitor : public libsinsp::filter::ast::base_expr_visitor {
-		visitor():
+		visitor(std::set<falco::load_result::warning_code>& warnings,
+		        std::set<falco::load_result::deprecated_field>& deprecated_fields):
 		        m_is_equality_check(false),
 		        m_last_node_is_unsafe_field(false),
-		        m_warnings(nullptr) {}
+		        m_warnings(&warnings),
+		        m_deprecated_fields(&deprecated_fields) {}
 		visitor(visitor&&) = default;
 		visitor& operator=(visitor&&) = default;
 		visitor(const visitor&) = delete;
@@ -58,6 +57,7 @@ class filter_warning_resolver {
 		bool m_is_equality_check;
 		bool m_last_node_is_unsafe_field;
 		std::set<falco::load_result::warning_code>* m_warnings;
+		std::set<falco::load_result::deprecated_field>* m_deprecated_fields;
 
 		void visit(libsinsp::filter::ast::value_expr* e) override;
 		void visit(libsinsp::filter::ast::list_expr* e) override;

userspace/engine/rule_loader.cpp

@@ -116,7 +116,7 @@ void rule_loader::context::init(const std::string& name,
 	m_locs.push_back(loc);
 }
 
-std::string rule_loader::context::as_string() {
+std::string rule_loader::context::as_string() const {
 	std::ostringstream os;
 
 	// All valid contexts should have at least one location.
@@ -142,7 +142,7 @@ std::string rule_loader::context::as_string() {
 	return os.str();
 }
 
-nlohmann::json rule_loader::context::as_json() {
+nlohmann::json rule_loader::context::as_json() const {
 	nlohmann::json ret;
 
 	ret["locations"] = nlohmann::json::array();
@@ -282,9 +282,13 @@ void rule_loader::result::add_error(load_result::error_code ec,
 void rule_loader::result::add_warning(load_result::warning_code wc,
                                       const std::string& msg,
                                       const context& ctx) {
-	warning warn = {wc, msg, ctx};
+	warnings.emplace_back(std::make_unique<warning>(wc, msg, ctx));
+}
 
-	warnings.push_back(warn);
+void rule_loader::result::add_deprecated_field_warning(load_result::deprecated_field df,
+                                                       const std::string& msg,
+                                                       const context& ctx) {
+	warnings.emplace_back(std::make_unique<deprecated_field_warning>(df, msg, ctx));
 }
 
 void rule_loader::result::set_schema_validation_status(const std::vector<std::string>& status) {
@@ -369,8 +373,7 @@ const std::string& rule_loader::result::as_summary_string() {
 			}
 			first = false;
 
-			os << load_result::warning_code_str(warn.wc) << " ("
-			   << load_result::warning_str(warn.wc) << ")";
+			os << warn->code_string() << " (" << warn->as_string() << ")";
 		}
 		os << "]";
 	}
@@ -438,14 +441,13 @@ const std::string& rule_loader::result::as_verbose_string(const rules_contents_t
 		os << warnings.size() << " Warnings:" << std::endl;
 
 		for(auto& warn : warnings) {
-			os << warn.ctx.as_string();
+			os << warn->ctx.as_string();
 
 			os << "------" << std::endl;
-			os << warn.ctx.snippet(contents);
+			os << warn->ctx.snippet(contents);
 			os << "------" << std::endl;
 
-			os << load_result::warning_code_str(warn.wc) << " ("
-			   << load_result::warning_str(warn.wc) << "): " << warn.msg;
+			os << warn->code_string() << " (" << warn->as_string() << "): " << warn->msg;
 			os << std::endl;
 		}
 	}
@@ -492,16 +494,7 @@ const nlohmann::json& rule_loader::result::as_json(const rules_contents_t& conte
 
 	j["warnings"] = nlohmann::json::array();
 	for(auto& warn : warnings) {
-		nlohmann::json jwarn;
-
-		jwarn["context"] = warn.ctx.as_json();
-		jwarn["context"]["snippet"] = warn.ctx.snippet(contents);
-
-		jwarn["code"] = load_result::warning_code_str(warn.wc);
-		jwarn["codedesc"] = load_result::warning_desc(warn.wc);
-		jwarn["message"] = warn.msg;
-
-		j["warnings"].push_back(jwarn);
+		j["warnings"].push_back(warn->as_json(contents));
 	}
 
 	res_json = j;