render_array_markup_xss reports every variable assigned to '#markup' as an XSS error, but the renderer filters strings; only Markup::create() values and raw user input are risky

Problem/Motivation

The static security rule render_array_markup_xss (modules/audit_security/rules/static_security_rules.yml) reports an ERROR ("Assigning a dynamic variable to '#markup' can lead to XSS.") for any '#markup' whose value starts with '$':

return ['#markup' => $this]; reported (a MarkupInterface value object)
$build = ['#markup' => $this->t('Safe string')]; reported (TranslatableMarkup)
$build['help'] = ['#markup' => $description]; reported (an HTML string built by the module)

But the renderer filters '#markup': unless the value is a MarkupInterface object, Renderer::ensureMarkupIsSafe() passes it through Xss::filter() with the admin tag list (or '#allowed_tags'). A string variable in '#markup' is therefore not raw output. The value that really skips filtering is one marked as safe with Markup::create(), and that one is MISSED by the current pattern ('#markup' => Markup::create($html) does not start with '$'), as is any expression that does not start with a variable ('Hi ' . $_GET['name']).

On a real site the security report had 55 RENDER_ARRAY_MARKUP_XSS errors, all of them in custom modules building their own help text or rendering translated strings, and none of them a real problem.

Steps to reproduce

1. Add $build = ['#markup' => $this->t('Hello')]; to a custom module.
2. Run drush audit:run security --format=json.
3. RENDER_ARRAY_MARKUP_XSS is reported as an error on that line.

Proposed resolution

Replace the rule with two narrower rules (all static rules are line-based, so both look at the same line):

1. render_array_markup_xss (error): '#markup' receives Markup::create() of something that is not a literal and not sanitized on the same line (Xss::filter*(), Html::escape(), t()). Pattern: ['"]#markup['"]\s*=>\s*Markup::create\(\s*(?!['"]|Xss::|Html::escape\(|(\$this->)?t\()
2. render_array_markup_input (warning, new): '#markup' receives request input ($_GET/$_POST/$_REQUEST/$_COOKIE, $request->, getRequest(), ->query->get(, ->request->get(, getUserInput() or a raw field value (->value). These are only filtered with the permissive admin tag list, so the HTML they may contain is decided by the visitor or the editor instead of a text format. Recommendation: '#plain_text', or render the field / '#type' => 'processed_text'.

Plain variables, $this and translated strings are no longer reported. Both messages explain that string '#markup' values are admin-filtered.

Remaining tasks

- Update the rules in static_security_rules.yml.
- Unit test loading the shipped rules: $this, $this->t(), an HTML string variable, Markup::create() of a literal and of Xss::filterAdmin() => not reported; Markup::create($html) => RENDER_ARRAY_MARKUP_XSS; $request->query->get(), a $_GET concatenation and $node->body->value => RENDER_ARRAY_MARKUP_INPUT.

User interface changes

RENDER_ARRAY_MARKUP_XSS is only reported for unsanitized Markup::create() values; new warning RENDER_ARRAY_MARKUP_INPUT.

API changes

None.

Data model changes

None. Sites that disabled RENDER_ARRAY_MARKUP_XSS through rule_overrides or audit-ignore markers keep that setting; the new rule has its own code.