Problem/Motivation

Static rules are all-or-nothing and global. audit.settings only offers scan_directories and exclude_patterns, which exclude whole files or directories from every rule. audit_security.settings only has trusted_roles, and audit_performance has no per-rule settings. Other submodules already show the need for per-rule toggles: audit_complexity has detect_service_locators, detect_deep_arrays, detect_hardcoded_ids and detect_direct_queries, audit_phpcs has min_severity and excluded_sniffs, and audit_views has cache_excluded_views.

Typical team policies that today require excluding a whole module directory: CACHE_MAX_AGE_ZERO as a notice for admin controllers; DIRECT_DB_CONNECTION disabled for migration or batch tooling modules; T_FUNCTION_XSS disabled site-wide because the team follows the '@' placeholder convention; QUERY_NO_RANGE disabled for search_api plugins.

This complements the inline audit-ignore marker, which is per finding in the code: this one is for policies decided by the team.

Proposed resolution

Add a rule_overrides setting to audit.settings, edited in the General Settings of the settings form, with one override per line:

# RULE_ID [PATH_PATTERN] SEVERITY
CACHE_MAX_AGE_ZERO */src/Controller/Admin* notice
DIRECT_DB_CONNECTION web/modules/custom/migrate_tools/* disabled
T_FUNCTION_XSS disabled
  • SEVERITY is error, warning, notice or disabled. A disabled rule is not reported; a changed severity is reported and scored at that level.
  • PATH_PATTERN is matched against the whole file path relative to the project root, like scan_directories. "*" matches any characters, "/" included, and "?" a single character. Without a pattern the override applies everywhere. When several lines match, the last one wins. Empty lines and lines starting with "#" are ignored.
  • The setting is stored as a string, like scan_directories and exclude_patterns, so the form and the configuration use the same format. The form rejects lines that are not valid overrides.
  • A new Drupal\audit\StaticAnalysis\RuleOverrides parses and applies the setting. AuditAnalyzerBase::getRuleOverrides() loads it once per analyzer, and it is applied by SecurityAnalyzer::scanStaticFile() and by all the per-file scanners of PerformanceAnalyzer, before the inline audit-ignore markers.
  • Schema, install default and audit_update_10006() for existing sites. README section.

Differences from the first idea for this issue: rule IDs are not validated against the list of known rules, since an unknown ID is simply never matched; and TwigAnalyzer is out of scope, like for the inline marker, because it works per template rather than per file and line (Twig debug patterns are reported by the performance audit, which honours the overrides).

Remaining tasks

  • Review the merge request.

User interface changes

New "Static rule overrides" textarea in the General Settings of the Audit settings form.

API changes

New Drupal\audit\StaticAnalysis\RuleOverrides and AuditAnalyzerBase::getRuleOverrides().

Data model changes

New audit.settings:rule_overrides string, added to existing sites by audit_update_10006().

Comments

trebormc created an issue. See original summary.

  • trebormc committed 64c2770a on 1.x
    Issue #3626580 by trebormc: Allow disabling a static rule or changing...

  • trebormc committed 704da1f4 on 1.x
    Issue #3626580 followup by trebormc: Describe scan_directories as...

  • trebormc committed cd7c1402 on 1.x
    Issue #3626580 followup by trebormc: Strip only the project root prefix...
trebormc’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.