Use #[SensitiveParameter] attribute where appropriate

[mbabker]

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):

• The $password argument of the Twilio\Base\BaseClient constructor and its request() method
• The $authToken argument of the Twilio\Jwt\ClientToken constructor
• The $password argument of the Twilio\Http\Client interface's request() method (and all implementations)
• The $authToken argument of the Twilio\Security\RequestValidator constructor

Given 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 a create() method with a $sipAuthPassword argument and setSipAuthPassword() 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:

<?php

namespace Twilio\Http;

interface Client {
    public function request(
        string $method,
        string $url,
        array $params = [],
        array $data = [],
        array $headers = [],
        string $user = null,
        #[\SensitiveParameter]
        string $password = null,
        int $timeout = null
    ): Response;
}

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.

[rakatyal]

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.

[TimWolla]

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.

[mbabker]

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, 👍

[TimWolla]

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.