Problem/Motivation

Static findings that are intentional cannot be acknowledged: an admin report with max-age 0 and no cache tags, a batch callback using \Drupal::database(), a loadMultiple() on a config entity storage, a t() with an '@' placeholder holding a trusted label. The only escape hatch is audit.settings:exclude_patterns (SecurityAnalyzer::getStaticExcludePatterns() / shouldExcludeStaticFile() and the equivalent parsing in PerformanceAnalyzer and TwigAnalyzer), which hides the whole file or directory from every rule. Site owners therefore either exclude entire modules, losing real findings, or live with a permanently degraded score, and the report stops being a to-do list.

There is no per-finding mechanism in the module today (no audit-ignore, suppression or acknowledged handling). #3606300 ("Add a flag to mark an audit resolved") is the UI-level counterpart at analyzer level; this request is per finding, in the code, and reviewable in version control, like @phpstan-ignore, phpcs:ignore or eslint-disable.

Proposed resolution

Honour an inline marker, written in any comment, in the static code scanners of the security and performance audits (PHP, Twig and YAML files):

// audit-ignore: CACHE_MAX_AGE_ZERO -- admin report backed by a log table.
'#cache' => ['max-age' => 0],

{# audit-ignore: TWIG_DEBUG_KINT -- documentation example #}
  • Syntax: audit-ignore: RULE_ID[, RULE_ID...] [-- reason]. It applies to its own line and to the next one, so it can trail the code or precede it. audit-ignore-next-line applies to the next line only, and audit-ignore-file to the whole file. Rule IDs are the finding codes shown in the report and are matched case-insensitively.
  • A new Drupal\audit\StaticAnalysis\SuppressionMap parses the markers from the raw file content, before comments are stripped for pattern matching, and is used by SecurityAnalyzer::scanStaticFile() and by all the per-file scanners of PerformanceAnalyzer (PHP/Twig patterns, performance patterns, *.libraries.yml and views YAML).
  • Acknowledged findings are not dropped. AuditAnalyzerBase::acknowledgeResultItem() turns them into informational items that keep their code and add the reason to the message and to details (acknowledged, acknowledged_reason). Following the existing convention for 'info' items, they are listed in the report but do not count towards the summary, the score or drush audit:run --fail-on.
  • The JSON summary gets an acknowledged count, also when the output is combined or filtered.
  • SecurityAnalyzer counted 'info' items as notices in its static code summary; that is aligned with the rest of the module. No static security rule uses 'info' today, so current scores do not change.

Out of scope for this issue: TwigAnalyzer, which works per template rather than per file and line (Twig debug patterns such as TWIG_DEBUG_KINT are already detected by the performance audit, which honours the marker); a separate collapsed "Acknowledged" section in the report; and a notice for markers without a reason. They can be follow-ups if needed.

Remaining tasks

  • Review the merge request.

User interface changes

Acknowledged findings are shown as informational items with "(acknowledged: reason)" appended to their message.

API changes

New Drupal\audit\StaticAnalysis\SuppressionMap and AuditAnalyzerBase::acknowledgeResultItem(). The Drush JSON summary gains an acknowledged key.

Comments

trebormc created an issue. See original summary.

  • trebormc committed 55b0e755 on 1.x
    Issue #3626576 by trebormc: Add an inline audit-ignore marker so...
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.