Skip to content

Look for the html-escaped spelling of a secret too - #2069

Open
youdie006 wants to merge 1 commit into
Seldaek:mainfrom
youdie006:redact-html-escaped-secrets
Open

youdie006 wants to merge 1 commit into
Seldaek:mainfrom
youdie006:redact-html-escaped-secrets

Conversation

@youdie006

Copy link
Copy Markdown

RedactingFormatter sweeps the wrapped formatter's output for the secret values it collected, and collectSecrets builds one escaped copy of each secret for that sweep (RedactingFormatter.php:176-184):

$escaped = substr(Utils::jsonEncode($secret, null, true), 1, -1);

That is the json spelling. HtmlFormatter escapes with htmlspecialchars instead (HtmlFormatter.php:61, :63, :76), so a secret containing &, < or > arrives in the output as &amp;, &lt;, &gt; - a spelling the needle list does not carry - and the sweep leaves it in place.

Measured with the same record and secret through both formatters:

LineFormatter  raw-present=false  html-escaped-present=false
HtmlFormatter  raw-present=false  html-escaped-present=true

This is the same shape as #2067, where the sweep also had nothing to match a value with. The fix adds the html spelling next to the json one, with the flags HtmlFormatter itself passes so the needle is byte-identical to what it emits. The existing !== '' guard still covers htmlspecialchars returning '' for invalid UTF-8, matching HtmlFormatter, which does not pass ENT_SUBSTITUTE either.

Verification

vendor/bin/phpunit --filter RedactingFormatterTest --no-coverage on php:8.3-cli. md5sum src/Monolog/Formatter/RedactingFormatter.php on every row.

source md5 result
this PR 9620c057 Assertions: 77, 0 failures
pristine 6ab54e26 1 failure, the new test
drop the html needle 38b9440e 1 failure, the new test
drop the json needle d09c6c26 1 failure, testRedactsSecretsNeedingJsonEscaping
ENT_NOQUOTES to ENT_QUOTES 467207f3 1 failure, the new test

Dropping either needle fails a different test, so both spellings are independently required. The flag row matters because a first draft used a secret with no quote in it and ENT_QUOTES passed; the committed test carries one, so the two flag choices produce different needles.

Full suite with --exclude-group Elasticsearch,Elastica,e2e,MongoDB: 2 failures against 3 on pristine, and the 14 errors are identical on both (missing ext-mongodb and ext-amqp, GitProcessorTest, SignalHandlerTest). phpstan analyse reports [OK] No errors. php -l clean on both files.

Not run: the suite on PHP 8.1, the composer.json floor, because the vendored PHPUnit needs 8.2.

Disclosure: this change was written with AI assistance (Claude). The measurements above were reproduced and checked by me before opening this PR.

RedactingFormatter sweeps the wrapped formatter's output for the secret
values it collected, and collectSecrets builds one escaped copy of each
secret for that sweep: the json spelling. HtmlFormatter escapes with
htmlspecialchars instead, so a secret containing &, < or > reaches the
output in a spelling the sweep does not carry and survives redaction.

This is the same shape as Seldaek#2067, where the sweep had nothing to match a
value with. Add the html spelling next to the json one, using the flags
HtmlFormatter itself passes so the needle matches what it emits.

addcookie on the response side and the other formatters are unaffected;
only the sweep's needle list changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant