WordPress / Requests

Requests for PHP is a humble HTTP request library. It simplifies how you interact with other sites and takes away all your worries.
https://requests.ryanmccue.info/
Other
3.57k stars 499 forks source link

Mark parameters containing user credentials as sensitive #878

Open jrfnl opened 4 months ago

jrfnl commented 4 months ago

PHP 8.2 introduced the SensitiveParameter attribute.

The effect of the attribute is that the value of the parameter is no longer directly shown in stack traces; instead, starting with PHP 8.2, the parameter will be presented as a SensitiveParameterValue object.

As the attribute only applies to parameters, it (unfortunately) has no effect on serialization of the object. See: https://3v4l.org/StoQO Might be an idea to start a discussion about an SensitiveProperty attribute on the PHP Internals mailing list, but that's outside the scope of this PR.

For now, this PR marks the $args parameter for the Auth\Basic class constructor and the Proxy\Http constructor as sensitive as both of these are supposed to contain user credentials (user name, password) for accessing a protected URL.

Includes updating the example code for custom authentication to also use the attribute.

Open question

The $options array passed to a large range of Requests methods can also contain credentials. Should this parameter also be marked as sensitive in all appropriate places ?

Refs:

schlessera commented 4 months ago

The $options array passed to a large range of Requests methods can also contain credentials. Should this parameter also be marked as sensitive in all appropriate places ?

Wrapping the entire $options array as a sensitive parameter would hinder a lot of the debugging efforts, so I think that might be too generic to block.

How about creating a polyfill for SensitiveParameterValue on PHP < 8.2 instead and wrapping only $options>auth and $options>proxy in that class?

See https://www.php.net/manual/en/class.sensitiveparametervalue.php

jrfnl commented 4 months ago

Let's think this over some more.