[jwrosewell]

Problem

specifications#55 (merged as c9939a78) added a "License keys" section to logging.md, and specifications#123 (247f071) has since changed its wording. At specifications 093bf3e5 it says at lines 20 to 24:

A license key MUST NOT appear in full in a log entry or in an error message, at any log level. Where a message names a license key it MUST show only the key's last 4 characters, with each character before them replaced by one *, so that a reader can tell two keys apart but cannot use either. A key of 4 characters or fewer MUST be replaced entirely, by one * for each character.

and at lines 32 to 39:

A message that names such a URL MUST name it with the key masked in the same way. The same applies to the message of an error raised by an HTTP library, which often includes the URL it requested.

There MUST be a test that a failed request carrying a license key produces log entries and error messages that hold only the masked form of the key.

No issue exists in this repository.

What I checked

I searched every PHP package for a place that logs or throws with a license key or a URL that carries one.

• DidClient (the only code that holds a license key) sends it in the form body, never in the URL. Its error messages carry text from the server's reply, which can repeat the key, and its transport can raise a message that names it.
• HttpClient in the Cloud Request Engine package puts the URL it requested into its error messages, and a URL can carry a license, licenseKey or licenseKeys query parameter, or the same with the licence spelling, which the cloud service also reads (cloud-request-engine.md, "The server also reads licence as another spelling of license"). The Cloud Request Engine in PHP sends only a resource key today.
• CloudRequestEngine::addQueryData warns with trigger_error when two pieces of Evidence give the same field, and the warning names the values, so it could name licence key Evidence such as query.license. #56, now merged, leaves Evidence named license or licence out of the request before that check.
• The Device Detection and Location pipeline builders declare a licenseKey property that nothing reads or logs. The on-premise engine's licence key is held by the native code, which this repository does not log.

Change

• New LicenseKey class in the core package (pipeline/core/src/LicenseKey.php) with mask() (one * for each hidden character and the last 4 characters shown, or all * for a key of 4 or fewer) and maskQuery(), which masks the value of each license, licenseKey and licenseKeys query parameter, in either spelling, anywhere in a text, including each key of a list separated by | or ,.
• HttpClient passes the failure messages and the error messages it raises through maskQuery().
• The Cloud Request Engine package now requires core ^4.5.207 (pipeline/cloudrequestengine/composer.json), because HttpClient calls LicenseKey, which no published core has. With 4.*, a site that updates only the Cloud Request Engine would keep core 4.5.206 and fail with "Class not found" on its first failed request. 4.5.207 is the version ci/get-next-package-version.ps1 gives the next release, one patch above the highest tag, 4.5.206, so the floor is right if this draft is in the next release from main. The packages that use the Cloud Request Engine get core through it. CI is not affected, because it installs core from this checkout as 4.x-dev.
• DidClient replaces its own license key, in plain and URL encoded form, in the messages it builds from a reply and in the message of a failure raised by the transport. The reply text is masked before it is cut to 200 characters, so a key cannot be left half hidden. The new exception for a transport failure has no previous exception, because that would carry the original message. The number of characters shown and the mask character are named constants.

Tests

• LicenseKeyTests in the core package, including the licence and LicenceKey spellings.
• HttpClientFailureTests::testAFailedRequestHoldsOnlyTheMaskedLicenseKey makes a request to a port nothing listens on, with a license key in the URL.
• Three tests in DidClientTest for a 400 reply, a 500 reply and a transport failure that name the key.

Runs.

• The masking. Red, without the change, run 42701274663. LicenseKeyTests 6 errors, HttpClientFailureTests 1 failure, DidClientTest 3 failures. Green run 42701302662, all three pass (6, 3 and 86 tests).
• The licence spelling. Red run 42796923319 on the test commit 524ed22, testMaskQueryMasksTheLicenceSpelling fails. Green run 42797120448 on 06abd7c, 284 tests, none failing.
• The core requirement. A temporary workflow installs the Cloud Request Engine from this branch on its own, with core and engines from Packagist, as a site would, and runs HttpClientFailureTests. Red run 42796955790 on 524ed22, where Composer installs core 4.5.206 and three tests fail with Class "fiftyone\pipeline\core\LicenseKey" not found. Green run 42797164599 on 06abd7c, where Composer refuses to install, because the newest published core is 4.5.206 and the package needs ^4.5.207.

Full build

Run 42885000600 on head 1b67dee passed every job, being Configure, the 8 build and test jobs (Ubuntu PHP 7.4 and 8.0 to 8.5, and Windows PHP 8.3) and Complete. Every one of the 9 Setting 'PullRequestSha' lines reads 1b67dee, and Configure printed "Left out 5 macOS entries".

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

Other drafts

• #58 is based on this draft, because it uses LicenseKey::mask, so merge this one first.
• This draft merged rc at 1fed97f after #48 landed, keeping both lines Set the client-side cache headers, with a weak ETag of the supplied Evidence #48 and this draft add to pipeline/core/phpunit.xml. Other open drafts add a line at the same place, and each keeps every line. After #51 landed, rc at 08f7bef was merged in (head 1b67dee), keeping both phpunit.xml lines, and the full build above is on that head. The head merges cleanly into rc at 0196662, which holds Add the web request to Evidence as a read-only view under server.http-request #49 and Count a sequence only as digits from 0 to 2,147,483,646, with no sign or spaces #59, and the full build above merged that rc.

Not covered

Every third-party transport a site might pass to DidClient. The contract says a transport throws RuntimeException, and only that type is masked. CloudException still carries the reply body as data, outside its message, which the page does not cover.

Remaining

Review by someone other than the author.