WIP on better error contexts

Change the semantics of lua load_rules to return a
successful/unsuccessful status instead of throwing errors when loading
fails. On success, load_rules returns [true, required_engine_version]
and on failure load_rules returns [false, row, col, error string]. The
row/col will be used to include context on error.

Falco's output itself now prints validation results to stdout, which
makes it easier to capture the output and pass it along. Log messages
continue to go to stderr.

Still need to finish going through load_rules and return all errors
directly instead of throwing a lua error.

userspace/engine/lua/rule_loader.lua

@@ -190,16 +190,32 @@ function load_rules(sinsp_lua_parser,
 		    replace_container_info,
 		    min_priority)
 
-   local rules = yaml.load(rules_content)
    local required_engine_version = 0
 
+   local status, rules = pcall(yaml.load, rules_content)
+
+   if status == false then
+      local pat = "^([%d]+):([%d]+): "
+      -- rules is actually an error string
+
+      local row = 0
+      local col = 0
+
+      row, col = string.match(rules, pat)
+      if row ~= nil and col ~= nil then
+	 rules = string.gsub(rules, pat, "")
+      end
+
+      return false, row, col, rules
+   end
+
    if rules == nil then
       -- An empty rules file is acceptable
-      return required_engine_version
+      return true, required_engine_version
    end
 
    if type(rules) ~= "table" then
-      error("Rules content \""..rules_content.."\" is not yaml")
+      return false, 1, 1, "Rules content is not yaml"
    end
 
    -- Iterate over yaml list. In this pass, all we're doing is
@@ -574,7 +590,7 @@ function load_rules(sinsp_lua_parser,
 
    io.flush()
 
-   return required_engine_version
+   return true, required_engine_version
 end
 
 local rule_fmt = "%-50s %s"

userspace/engine/rules.cpp

@@ -44,8 +44,7 @@ falco_rules::falco_rules(sinsp* inspector,
 			 lua_State *ls)
 	: m_inspector(inspector),
 	  m_engine(engine),
-	  m_ls(ls),
-	  m_err_linecol_re("^(\\d+):(\\d+)")
+	  m_ls(ls)
 {
 	m_sinsp_lua_parser = new lua_parser(engine->sinsp_factory(), m_ls, "filter");
 	m_json_lua_parser = new lua_parser(engine->json_factory(), m_ls, "k8s_audit_filter");
@@ -214,11 +213,10 @@ std::string falco_rules::get_context(const std::string &content, uint64_t lineno
 	std::string line;
 	std::string ret;
 
-	ret += "---\n";
 	for(uint32_t i=1; ctx && i<=lineno;  i++)
 	{
 		getline(ctx, line);
-		if(i > (lineno-3))
+		if((lineno - i) < 3)
 		{
 			ret += line + "\n";
 		}
@@ -229,7 +227,6 @@ std::string falco_rules::get_context(const std::string &content, uint64_t lineno
 		ret += " ";
 	}
 	ret += "^\n";
-	ret += "---\n";
 
 	return ret;
 }
@@ -454,28 +451,41 @@ void falco_rules::load_rules(const string &rules_content,
 		lua_pushstring(m_ls, extra.c_str());
 		lua_pushboolean(m_ls, (replace_container_info ? 1 : 0));
 		lua_pushnumber(m_ls, min_priority);
-		if(lua_pcall(m_ls, 9, 1, 0) != 0)
+		if(lua_pcall(m_ls, 9, 4, 0) != 0)
 		{
 			const char* lerr = lua_tostring(m_ls, -1);
 
 			string err = "Error loading rules: " + string(lerr);
 
-			// The lua error might start with a line
-			// number and column number. If it does, add
-			// context to the error.
-			std::smatch match;
-			string lerr_str = lerr;
-			if(regex_search(lerr_str, match, m_err_linecol_re) && match.size() > 1)
+			throw falco_exception(err);
+		}
+
+		// Either returns (true, required_engine_version), or (false, row, col, error string)
+		bool successful = lua_toboolean(m_ls, -4);
+
+		if(successful)
+		{
+			required_engine_version = lua_tonumber(m_ls, -3);
+		}
+		else
+		{
+			int line = lua_tonumber(m_ls, -3);
+			int col = lua_tonumber(m_ls, -2);
+			std::string err = lua_tostring(m_ls, -1);
+
+			// If the line/columns are >= 0, use them to
+			// extract meaninful context from the result.
+			if(line >= 1 && col >= 1)
 			{
 				err += "\n";
-				err += get_context(rules_content, atoi(match.str(1).c_str()), atoi(match.str(2).c_str()));
+				err += get_context(rules_content, line, col);
 			}
 
 			throw falco_exception(err);
 		}
 
-		required_engine_version = lua_tonumber(m_ls, -1);
-		lua_pop(m_ls, 1);
+		lua_pop(m_ls, 4);
+
 	} else {
 		throw falco_exception("No function " + m_lua_load_rules + " found in lua rule module");
 	}

userspace/engine/rules.h

@@ -65,7 +65,6 @@ class falco_rules
 	sinsp* m_inspector;
 	falco_engine *m_engine;
 	lua_State* m_ls;
-	std::regex m_err_linecol_re;
 	string m_lua_load_rules = "load_rules";
 	string m_lua_ignored_syscalls = "ignored_syscalls";
 	string m_lua_ignored_events = "ignored_events";

userspace/falco/falco.cpp

@@ -716,7 +716,17 @@ int falco_init(int argc, char **argv)
 			}
 			for(auto file : validate_rules_filenames)
 			{
-				engine->load_rules_file(file, verbose, all_events);
+				// Only include the prefix if there is more than one file
+				std::string prefix = (validate_rules_filenames.size() > 1 ? file + ": " : "");
+				try {
+					engine->load_rules_file(file, verbose, all_events);
+				}
+				catch(falco_exception &e)
+				{
+					printf("%s%s", prefix.c_str(), e.what());
+					throw;
+				}
+				printf("%sOk\n", prefix.c_str());
 			}
 			falco_logger::log(LOG_INFO, "Ok\n");
 			goto exit;