feat: add tag linter. (#304)

Add a tag linter that can be configured to use either a list of allowed strings,
or a regexp. If both are provided in the config file the allowed list is
preferred by the "check" command as it is more explicit.

Author: wxsBSD

cli/src/commands/check.rs

@@ -182,6 +182,21 @@ pub fn exec_check(
                     linters::rule_name(re)?.error(config.rule_name.error));
             }
 
+            // Prefer allowed list over the regex, as it is more explicit.
+            if !config.tags.allowed.is_empty() {
+                compiler.add_linter(
+                    linters::tags_allowed(config.tags.allowed.clone())
+                    .error(config.tags.error));
+            } else if let Some(re) = config
+                .tags
+                .regexp
+                .as_ref()
+                .filter(|re| !re.is_empty()) {
+                compiler.add_linter(
+                    linters::tag_regex(re)?.error(config.tags.error)
+                );
+            }
+
             compiler.colorize_errors(io::stdout().is_tty());
 
             match compiler.add_source(src) {

cli/src/config.rs

@@ -69,30 +69,47 @@ pub enum MetaValueType {
     Hash,
 }
 
-/// Format specific configuration information.
+/// Linter specific configuration information.
 #[derive(Deserialize, Serialize, Debug, Default)]
 #[serde(deny_unknown_fields)]
 pub struct CheckConfig {
-    /// Meta specific formatting information.
+    /// Meta specific linting information.
     // Note: Using a BTreeMap here because we want a consistent ordering when
     // we iterate over it, so that warnings always appear in the same order.
     pub metadata: BTreeMap<String, MetadataConfig>,
-    /// Rule name formatting information.
+    /// Rule name linting information.
     pub rule_name: RuleNameConfig,
+    /// Tag linting information.
+    pub tags: TagConfig,
 }
 
-/// Format specific configuration information.
+/// Allowed tag names in the linter.
+#[derive(Deserialize, Serialize, Debug, Default)]
+#[serde(deny_unknown_fields)]
+pub struct TagConfig {
+    /// List of allowed tags.
+    pub allowed: Vec<String>,
+    /// Regexp that must match all tags.
+    pub regexp: Option<String>,
+    /// If `true`, an incorrect tag name will raise an error instead of a
+    /// warning.
+    #[serde(default)]
+    pub error: bool,
+}
+
+/// Rule name linter specific configuration information.
 #[derive(Deserialize, Serialize, Debug, Default)]
 #[serde(deny_unknown_fields)]
 pub struct RuleNameConfig {
     /// Regexp used to validate the rule name.
     pub regexp: Option<String>,
-    /// If `true`, an incorrect rule anme will raise an error instead of a
+    /// If `true`, an incorrect rule name will raise an error instead of a
     /// warning.
     #[serde(default)]
     pub error: bool,
 }
 
+///  Metadata linter specific configuration information.
 #[derive(Deserialize, Serialize, Debug)]
 #[serde(deny_unknown_fields)]
 pub struct MetadataConfig {

cli/src/tests/check.rs

@@ -153,7 +153,7 @@ fn check_rule_name_error() {
         .success()
         .stdout(
             r#"[ FAIL ] src/tests/testdata/foo.yar
-error[E038]: rule name does not match regex `APT_.+`
+error[E039]: rule name does not match regex `APT_.+`
  --> src/tests/testdata/foo.yar:1:6
   |
 1 | rule foo : bar baz {
@@ -187,7 +187,7 @@ fn config_error() {
         .failure()
         .stderr(
             predicate::str::contains(
-                r#"error: unknown field: found `foo`, expected ``metadata` or `rule_name`` for key "default.check.foo""#,
+                r#"error: unknown field: found `foo`, expected `one of `metadata`, `rule_name`, `tags`` for key "default.check.foo""#,
             )
         );
 }

lib/src/compiler/errors.rs

@@ -68,6 +68,7 @@ pub enum CompileError {
     InvalidRegexp(Box<InvalidRegexp>),
     InvalidRegexpModifier(Box<InvalidRegexpModifier>),
     InvalidRuleName(Box<InvalidRuleName>),
+    InvalidTag(Box<InvalidTag>),
     InvalidUTF8(Box<InvalidUTF8>),
     MethodNotAllowedInWith(Box<MethodNotAllowedInWith>),
     MismatchingTypes(Box<MismatchingTypes>),
@@ -84,6 +85,7 @@ pub enum CompileError {
     UnknownIdentifier(Box<UnknownIdentifier>),
     UnknownModule(Box<UnknownModule>),
     UnknownPattern(Box<UnknownPattern>),
+    UnknownTag(Box<UnknownTag>),
     UnusedPattern(Box<UnusedPattern>),
     WrongArguments(Box<WrongArguments>),
     WrongType(Box<WrongType>),
@@ -685,7 +687,7 @@ pub struct InvalidMetadata {
 /// ## Example
 ///
 /// ```text
-/// warning[missing_metadata]: required metadata is missing
+/// error[E038]: required metadata is missing
 ///  --> test.yar:12:6
 ///    |
 /// 12 | rule pants {
@@ -716,7 +718,7 @@ pub struct MissingMetadata {
 /// ## Example
 ///
 /// ```text
-/// warning[invalid_rule_name]: rule name does not match regex `APT_.*`
+/// error[E039]: rule name does not match regex `APT_.*`
 ///  --> test.yar:13:6
 ///    |
 /// 13 | rule pants {
@@ -726,7 +728,7 @@ pub struct MissingMetadata {
 #[derive(ErrorStruct, Clone, Debug, PartialEq, Eq)]
 #[associated_enum(CompileError)]
 #[error(
-    code = "E038",
+    code = "E039",
     title = "rule name does not match regex `{regex}`"
 )]
 #[label(
@@ -739,6 +741,68 @@ pub struct InvalidRuleName {
     regex: String,
 }
 
+/// Unknown tag. This is only used if the compiler is configured to check
+/// for required tags (see:  [`crate::linters::Tags`]).
+///
+/// ## Example
+///
+/// ```text
+/// error[E040]: tag not in allowed list
+///  --> rules/test.yara:1:10
+///   |
+/// 1 | rule a : foo {
+///   |          ^^^ tag `foo` not in allowed list
+///   |
+///   = note: Allowed tags: test, bar
+/// ```
+#[derive(ErrorStruct, Clone, Debug, PartialEq, Eq)]
+#[associated_enum(CompileError)]
+#[error(
+    code = "E040",
+    title = "tag not in allowed list"
+)]
+#[label(
+    "tag `{name}` not in allowed list",
+    tag_loc
+)]
+#[footer(note)]
+pub struct UnknownTag {
+    report: Report,
+    tag_loc: CodeLoc,
+    name: String,
+    note: Option<String>,
+}
+
+/// Tag does not match regex. This is only used if the compiler is configured to
+/// check for it (see: [`crate::linters::Tags`]).
+///
+/// ## Example
+///
+/// ```text
+/// error[E041]: tag does not match regex `bar`
+///  --> rules/test.yara:1:10
+///   |
+/// 1 | rule a : foo {
+///   |          ^^^ tag `foo` does not match regex `bar`
+///   |
+/// ```
+#[derive(ErrorStruct, Clone, Debug, PartialEq, Eq)]
+#[associated_enum(CompileError)]
+#[error(
+    code = "E041",
+    title = "tag does not match regex `{regex}`"
+)]
+#[label(
+    "tag `{name}` does not match regex `{regex}`",
+    tag_loc
+)]
+pub struct InvalidTag {
+    report: Report,
+    tag_loc: CodeLoc,
+    name: String,
+    regex: String,
+}
+
 /// A custom error has occurred.
 #[derive(ErrorStruct, Clone, Debug, PartialEq, Eq)]
 #[associated_enum(CompileError)]

lib/src/compiler/linters.rs

@@ -37,6 +37,7 @@ pub(crate) trait LinterInternal {
 pub(crate) enum LinterResult {
     Ok,
     Warn(Warning),
+    Warns(Vec<Warning>),
     Err(CompileError),
 }
 
@@ -114,6 +115,145 @@ impl LinterInternal for RuleName {
 
 type Predicate<'a> = dyn Fn(&Meta) -> bool + 'a;
 
+/// A linter that ensures tags meet specified requirements in either an allowed
+/// list of tags or in a regex.
+///
+/// ```
+/// # use yara_x::Compiler;
+/// use yara_x::linters;
+/// let mut compiler = Compiler::new();
+/// let warnings = compiler
+///     .add_linter(linters::tags_allowed(vec!["foo".to_string(), "bar".to_string()]))
+///     // This produces a warning because the rule tags are not from the
+///     // allowed list
+///     .add_source(r#"rule foo : test { strings: $foo = "foo" condition: $foo }"#)
+///     .unwrap()
+///     .warnings();
+///
+/// assert_eq!(
+///     warnings[0].to_string(),
+///     r#"warning[unknown_tag]: tag not in allowed list
+///  --> line:1:12
+///   |
+/// 1 | rule foo : test { strings: $foo = "foo" condition: $foo }
+///   |            ---- tag `test` not in allowed list
+///   |
+///   = note: allowed tags: foo, bar"#);
+pub struct Tags {
+    allow_list: Vec<String>,
+    regex: Option<String>,
+    compiled_regex: Option<Regex>,
+    error: bool,
+}
+
+impl Tags {
+    /// A list of strings that tags for each rule must match one of.
+    pub(crate) fn from_list(list: Vec<String>) -> Self {
+        Self {
+            allow_list: list,
+            regex: None,
+            compiled_regex: None,
+            error: false,
+        }
+    }
+
+    /// Regular expression that tags for each rule must match.
+    pub(crate) fn from_regex<R: Into<String>>(
+        regex: R,
+    ) -> Result<Self, regex::Error> {
+        let regex = regex.into();
+        let compiled_regex = Some(Regex::new(regex.as_str())?);
+        let tags = Self {
+            allow_list: Vec::new(),
+            regex: Some(regex),
+            compiled_regex,
+            error: false,
+        };
+        Ok(tags)
+    }
+
+    /// Specifies whether the linter should produce an error instead of a
+    /// warning.
+    ///
+    /// By default, the linter raises warnings about tags that don't match the
+    /// regular expression. This setting allows turning such warnings into
+    /// errors.
+    pub fn error(mut self, yes: bool) -> Self {
+        self.error = yes;
+        self
+    }
+}
+
+impl LinterInternal for Tags {
+    fn check(
+        &self,
+        report_builder: &ReportBuilder,
+        rule: &ast::Rule,
+    ) -> LinterResult {
+        if rule.tags.is_none() {
+            return LinterResult::Ok;
+        }
+
+        let mut results: Vec<Warning> = Vec::new();
+        let tags = rule.tags.as_ref().unwrap();
+        if !self.allow_list.is_empty() {
+            for tag in tags.iter() {
+                if !self.allow_list.contains(&tag.name.to_string()) {
+                    if self.error {
+                        return LinterResult::Err(errors::UnknownTag::build(
+                            report_builder,
+                            tag.span().into(),
+                            tag.name.to_string(),
+                            Some(format!(
+                                "allowed tags: {}",
+                                self.allow_list.join(", ")
+                            )),
+                        ));
+                    } else {
+                        results.push(warnings::UnknownTag::build(
+                            report_builder,
+                            tag.span().into(),
+                            tag.name.to_string(),
+                            Some(format!(
+                                "allowed tags: {}",
+                                self.allow_list.join(", ")
+                            )),
+                        ));
+                    }
+                }
+            }
+        } else {
+            let compiled_regex = self.compiled_regex.as_ref().unwrap();
+
+            for tag in tags.iter() {
+                if !compiled_regex.is_match(tag.name) {
+                    if self.error {
+                        return LinterResult::Err(errors::InvalidTag::build(
+                            report_builder,
+                            tag.span().into(),
+                            tag.name.to_string(),
+                            self.regex.as_ref().unwrap().clone(),
+                        ));
+                    } else {
+                        results.push(warnings::InvalidTag::build(
+                            report_builder,
+                            tag.span().into(),
+                            tag.name.to_string(),
+                            self.regex.as_ref().unwrap().clone(),
+                        ));
+                    }
+                }
+            }
+        }
+
+        if results.is_empty() {
+            LinterResult::Ok
+        } else {
+            LinterResult::Warns(results)
+        }
+    }
+}
+
 /// A linter that validates metadata entries.
 ///
 /// ```
@@ -286,6 +426,17 @@ impl LinterInternal for Metadata<'_> {
     }
 }
 
+/// Creates a tag linter from a list of allowed tags.
+pub fn tags_allowed(list: Vec<String>) -> Tags {
+    Tags::from_list(list)
+}
+
+/// Creates a tag linter that makes sure that each tag matches the given regular
+/// expression.
+pub fn tag_regex<R: Into<String>>(regex: R) -> Result<Tags, Error> {
+    Tags::from_regex(regex)
+}
+
 /// Creates a linter that validates metadata entries.
 ///
 /// See [`Metadata`] for details.

lib/src/compiler/mod.rs

@@ -1189,6 +1189,11 @@ impl Compiler<'_> {
                 LinterResult::Warn(warning) => {
                     self.warnings.add(|| warning);
                 }
+                LinterResult::Warns(warnings) => {
+                    for warning in warnings {
+                        self.warnings.add(|| warning);
+                    }
+                }
                 LinterResult::Err(err) => return Err(err),
             }
         }