Open mbabker opened 1 year ago
This issue has been added to our internal backlog (reference DI-1428) to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.
First, a polyfill of the attribute will be needed for older PHP versions
This is false. Attributes are not required to be backed by a class (you can use an arbitrary name), thus there is no need for a polyfill to be added if you just want to protect your parameters.
Huh, TIL then. I figured it would have to be there to avoid a class not found error if someone's doing something with attributes, but if not, 👍
I figured it would have to be there to avoid a class not found error if someone's doing something with attributes, but if not, +1
The class needs to exist for ReflectionAttribute::newInstance()
. However for SensitiveParameter
that is not useful, because it is parameterless anyway. In any case code that wants to use newInstance()
with SensitiveParameter
can just include the polyfill itself.
PHP 8.2 introduced a
SensitiveParameter
attribute which allows flagging method parameters as sensitive data, which instructs the engine to redact them when necessary (such as in stack traces), see https://php.watch/versions/8.2/backtrace-parameter-redaction for more information.The SDK should be updated to support this attribute where necessary. This should include (but is not limited to, this is only based on a scan in one of my active projects):
$password
argument of theTwilio\Base\BaseClient
constructor and itsrequest()
method$authToken
argument of theTwilio\Jwt\ClientToken
constructor$password
argument of theTwilio\Http\Client
interface'srequest()
method (and all implementations)$authToken
argument of theTwilio\Security\RequestValidator
constructorGiven that attributes are a PHP 8 feature, with the
SensitiveParameter
attribute only being introduced in PHP 8.2, and the SDK still supporting PHP 7.1, there are some extra steps needed to support this.First, a polyfill of the attribute will be needed for older PHP versions, Symfony provides a number of polyfill packages and its PHP 8.2 polyfill includes a polyfill for this class.
Second, for autogenerated code, the code generator would need to be taught how to mark sensitive parameters as, well, sensitive. Given there are constructors and setters for password parameters (i.e. the
Twilio\Rest\Api\V2010\Account\CallOptions
auto-generated class has acreate()
method with a$sipAuthPassword
argument andsetSipAuthPassword()
method), this would need to be identified. Note that this may be a PHP-specific feature, according to research done during the RFC discussion, other languages don't appear to dump parameters in contexts such as stack traces in the same way that PHP does.Lastly, non-autogenerated code would need to be updated to use the attribute. The attribute syntax was deliberately chosen as it will be parsed as a comment line on PHP 7 when the code is formatted properly. The
Twilio\Http\Client
interface would therefore need to look something like this:The attribute would apply to the
$password
argument, and the parser on PHP 7 would treat the line with the attribute as a comment and ignore it.