Store condition values as typed; remove the unused geolocation license key (#60, #54)

[sean-e-dietrich]

Two changes the maintainer decided on.

Fixes #60
Fixes #54

Summary

Condition values are stored as typed (#60)

Where it broke. Screen::posted_array() passes every posted setting through sanitize_deep(), which calls sanitize_textarea_field(). That function turns < into < (or strips it along with whatever follows) and deletes every %xx sequence. #59 worked around it only for fields marked verbatim. So condition values on every condition rule type, every field of a referenced list, and rate-limit lines were still rewritten on each save. The same sanitiser sat in front of the pasted document on the Import and Rules screens and the Advanced YAML box.

What changed.

• Rule_Edit_Screen::with_typed_conditions() re-reads the conditions and sources subtrees from $_POST. It applies wp_unslash() and Screen::typed() (removes C0 control characters and DEL, keeps tabs, keeps line breaks only in a list's multi-line advanced block). Keys still go through sanitize_key(). Rate-limit paths is now verbatim, and verbatim fields go through typed() as well.
• Condition_Rule_Type_Base::validate_settings() makes the decisions, for every writer (screen, import, WP-CLI): it drops control characters, refuses invalid UTF-8 and values over 4,096 characters, and still checks the operator, the variable and that a regex compiles.
• The Import, Rules and Advanced screens read their documents with the new posted_typed_textarea(). The Test screen reads path, user agent, headers and body with posted_typed() / posted_typed_textarea(), so it tests the request that was typed.
• Output escaping audit. Condition values and list fields go out through Screen::text() inside wp_kses(). Textareas use esc_textarea(). Selects, the rule-list summary, notices, the import preview and the Test screen's result use esc_html() / esc_attr(). Every one of these paths escapes. One round-trip bug turned up along the way: esc_attr() does not double-encode, so a stored value of <x was rendered as value="<x", the browser read it as <x, and the next save changed it. text() and the option values in select() now use htmlspecialchars(..., double_encode: true).

The geolocation license key is removed (#54)

• The field is gone from the geolocation/ASN reader settings, along with its default, its validation and the reader.license_key secret path on both types. Its removal from reader_help() also takes away the secret-field handling, which was generic. license_key stays in the generic Advanced YAML credential-name list and in the compiled-file key list.
• Schema 13 (Upgrader routine 13) removes license_key from the reader of every stored geolocation and ASN rule. It writes the settings only if it removed something, so it is idempotent. Referenced lists never held the key, so they need no migration.
• An imported document that still carries the key is imported without it. The preview (new settings_dropped summary key) and wp basic-firewall import name each rule the key was dropped from. The import does not fail.
• README updated. readme.txt never mentioned the key.

Testing

• New tests/integration/ConditionValueRoundTripTest.php:

  • Render the screen, post exactly what was rendered (parsed as HTML, the way a browser reads it), save twice, and compare what is stored. Values: <script>…, >, %2e%2e/, %3Cscript, &, <…>, quotes, backslashes, unicode, tab, and a regex (<|%3c)script\s*>. This runs for URL (header and query), user agent, geolocation, ASN and edge signal.
  • The rendered page and the rule list contain no raw <script.
  • Typed values keep everything except control characters. An invalid regex and an over-long value are refused.
  • List fields (percent-encoded URL, name with <>&", advanced where) and a percent-encoded rate-limit pattern come back unchanged.
  • Pasted Import and Advanced YAML documents are held or stored byte for byte.
  • Against the real library through Request_Tester: a saved header.x-probe contains %3Cscript blocks X-Probe: q=%3Cscript%3E and lets X-Probe: a script tag through. Through the sanitiser the rule became contains script, which matches both. The regex matches both <script> and %3cscript. A header is used because 2.35 normalises unreserved percent-encodings in path (%2e is . before a condition sees it) and Symfony decodes query.* values.
  • All 13 tests fail on main and pass here.

• UpgradeRoutinesTest::test_the_license_key_is_deleted_on_upgrade: keys are removed from geolocation and ASN rules, everything else is kept, and a second run writes nothing.

• GeoReaderSettingsTest: the field is not rendered, an import drops the key with a note (the key is never echoed), there is no note without a key, and the key is not a declared credential. Existing license-key tests were updated: RuleCredentialFieldsTest now covers only the AbuseIPDB key.

• Local: unit (326), composer analyse, composer lint, integration (639, 19 skipped, 2 pre-existing short-DB_PASSWORD warnings), e2e (13), tests/cli/commands.sh (36), build/check-versions.sh, build/build-zip.sh. The site's basic_firewall_settings are byte-identical before and after. The schema version is now 13.

• CircleCI: green.

• Was AI used in this pull request?