Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add tag linter. #304

Merged
merged 9 commits into from
Feb 12, 2025
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
15 changes: 15 additions & 0 deletions cli/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
27 changes: 22 additions & 5 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions cli/src/tests/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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""#,
)
);
}
70 changes: 67 additions & 3 deletions lib/src/compiler/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>),
Expand All @@ -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>),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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(
Expand All @@ -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)]
Expand Down
151 changes: 151 additions & 0 deletions lib/src/compiler/linters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub(crate) trait LinterInternal {
pub(crate) enum LinterResult {
Ok,
Warn(Warning),
Warns(Vec<Warning>),
Err(CompileError),
}

Expand Down Expand Up @@ -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.
///
/// ```
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions lib/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down
Loading
Loading