Repository navigation
Show a license key only by its last 4 characters in logs and error messages - #85
Merged
Merged
Conversation
A license key in a log entry or an error message is shown only by its last 4 characters, with every character before them replaced by '*', so a reader can tell two keys apart but cannot use either. A key of 4 characters or fewer is replaced in full, because its last 4 characters would be all of it. maskIn masks each key in a text, as written and as encoded in a URL, and gives a throwable to log in place of one whose message, causes or suppressed throwables hold a key, keeping the class names and stack traces.
The data update service is tested for a 403, a 429, another failing status, a 304, no response and an exception that names the URL, with two keys in the update URL. Everything it passes to the logger, at every level and with each exception's stack trace, holds each key only by its last 4 characters. The cloud request engine is tested with an error from the cloud that names the license key, and the 51Did client with a 401 body and a 400 errors list that name the licence, and with a key of 4 characters, which is replaced in full.
…errors The data update service logged the formatted update URL, whose query carries the license keys, at debug and error level, and logged the exception an HTTP library raised, whose message often names that URL. Every message now names the URL with each configured key masked by LicenseKeyMask, and the exception is logged as a copy whose messages are masked. The cloud answers a refused license key with an error that names it, and the cloud request engine put that text in the exception it raised, which can reach the log and the JSON builder's errors list. Its messages now mask the license key, and each key in it when it holds several separated by '|'. The 51Did client puts the start of the body in the message of a redeem error, and the errors list of a 400 answer in the message of the IllegalArgumentException it raises. Both now mask the licence key the redeem request sent. The 51Did package depends on nothing in the Pipeline, so it masks with a private method that gives the same result as LicenseKeyMask. The exception's body is kept as the service sent it.
…e URL maskIn_EncodedKeyShowsThePlainLast4 expects a key whose last characters are percent-encoded in a URL to show the encoded form of its plain last 4 characters. maskIn_MasksAKeyEncodedInLowerCase expects a key encoded with lower case hexadecimal digits to be masked too. KeyTypedIntoTheUrl_LogsMaskedKey gives the data update service a URL whose LicenseKeys parameter holds a key that is not among the data update license keys, and expects the key masked in every log entry. All three are written to fail on the previous commit.
… URL LicenseKeyMask.maskIn now replaces a URL-encoded key with the encoded form of its masked plain form, so it shows the same last 4 characters as the plain key, and also masks the key encoded with lower case hexadecimal digits. Its Javadoc now says that every occurrence of a key's text is masked, so a key of only a few characters masks those characters elsewhere too, as the specification asks for keys of 4 characters or fewer. The specification link now points at rc, where the rule is. The data update service now also masks any key in the LicenseKeys parameter of the data update URL, such as one typed straight into the DataUpdateUrl setting rather than given as a data update license key. DidHttpException.getBody now says the body is not masked and can hold the license key sent, so a caller that logs it must mask it first.
licenseKeysToMask was added between downloadFile and its Javadoc, so the Javadoc described the wrong method. It is now above downloadFile again.
This was referenced Nov 10, 2026
jwrosewell
marked this pull request as ready for review
November 10, 2026 10:18
This was referenced Nov 10, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #10. Follows the License keys section of the logging specification, which 51Degrees/specifications#55 added and #123 reworded, both now on
rc.The problem
A license key is a paid credential, and log entries and error messages travel further than the server, to log services, to support tickets and, through the JSON builder's
errorslist, to the browser. Onrcat 7299688 three places wrote license keys in full.FiftyOneUrlFormatterputs the license keys in the query of the update URL.DataUpdateServiceDefault.downloadFilelogged that URL at debug level for every download and for a 304, at error level for no response, a 429, a 403 and any other failing status, and logged any exception with its stack trace. The exception an HTTP library raises often names the URL it requested, for exampleServer returned HTTP response code: 403 for URL: ....CloudRequestEngineDefault.validateResponseput the cloud's error text, or the whole response for a failing status, in the message of theCloudRequestExceptionit raised.DidClient.readRedeemput the first 200 characters of the body in the message of a redeem error, and theerrorslist of a 400 answer in the message of theIllegalArgumentExceptionit raised. The redeem request is the one that sends the licence key.A search of every
logger.call and exception message inpipeline,device-detection,ip-intelligence,locationandexamplesfor license key values and for URLs or response bodies that can hold one found no other place.What the specification says
The License keys section of
pipeline-specification/features/logging.md, onrcat 5a25065, says the following.#55 also added to the logging section of
data-updates.mdthat the<url>in its messages "is the URL with each license key masked to its last 4 characters".What changed
fiftyone.pipeline.util.LicenseKeyMaskinpipeline.core. Its Javadoc links the specification onrc, where the rule is.mask(key)replaces every character before the last 4 with*, soTESTKEYNOTREALABCDEFGHIJ1234becomes************************1234. A key of 4 characters or fewer becomes all*.maskIn(text, keys)masks each key wherever the text holds it, as written and as encoded in a URL with upper or lower case hexadecimal digits, longest key first. An encoded key is replaced by the encoded form of its masked plain form, so it shows the same last 4 characters as the plain key, for exampleABCDEFGHIJKLMNOPQRST12%2B%3Dbecomes********************12%2B%3D.|list, therefore also masks those characters wherever else they appear. The specification asks for a key of 4 characters or fewer to be masked in full, so the helper keeps doing that, and its Javadoc says so.maskIn(throwable, keys)gives the throwable to log in place of one whose message, causes or suppressed throwables hold a key. Each message in the copy starts with the name of the class it stands for, and the stack traces are the originals. A throwable that holds no key is returned as it is, so logs that never held a key do not change.downloadFilenames the URL in every message with each key masked, and logs the exception throughmaskIn. The request itself still uses the unmasked URL. The keys are thoselicenseKeysToMaskgives, being the data update license keys and any key in the URL'sLicenseKeysparameter, so a key typed straight into theDataUpdateUrlsetting is masked too.validateResponsemasks the license key in every message before it raises the exception, and so does each key in it when the value holds several separated by|(getLicenseKeys).readRedeemmasks the licence key in both messages, before the body is cut to 200 characters, so a key cannot be left half shown.DidHttpException.getBody()still returns the body as the service sent it, and its Javadoc now says the body is not masked and can hold the license key sent, so a caller that logs it must mask it first. The 51Did package depends on nothing in the Pipeline butorg.json, so it cannot useLicenseKeyMaskwithout bringingpipeline.coreand its dependencies to every 51Did user. It masks with a private method of eight lines that gives the same result, and its tests pin the same masked value as the helper's tests.Two things are left as they are.
logging.mdcovers license keys only, so resource keys, such as the one in the URL of the accessible properties request, are not masked.CloudRequestExceptionkeeps the response's header fields as the cloud service sent them. The cloud request engine sends the license key only in the form body (getContent), never in a request header or URL, and nothing shows the service writing it into a response header. So there is nothing there to mask.For the release notes
*.fiftyone.pipeline.util.LicenseKeyMaskmasks license keys in text and in throwables, for code of your own that logs them.DataUpdateUrlsetting, in itsLicenseKeysparameter, is masked in the data update service's log as well.How to verify
No test calls a live service.
DidClientTestsneeds thepipeline/owid-javasubmodule checked out, as the 51Did package compiles its source.rcLicenseKeyMaskTests(pipeline.core)DataUpdateServiceLicenseKeyTests(pipeline.engines)CloudRequestEngineTests(pipeline.cloudrequestengine)DidClientTests(pipeline.did)CloudRequestEngineTestshas four parameterised methods, so a run reports more cases than methods.LicenseKeyMaskTests, which pass.rc. Ten are written to fail there: the six inDataUpdateServiceLicenseKeyTests(Forbidden,TooManyRequests,OtherStatus,NotModified,NoResponseandRequestException, each_LogsMaskedKeys),CloudRequestEngineTests.Process_LicenseKeyInCloudError_Masked, and inDidClientTestsredeem_RefusedLicenceIsMaskedInTheMessage,redeem_LicenceInErrorsIsMaskedInTheMessageandredeem_ShortLicenceIsMaskedInFull. The data update tests capture everything passed to a mocked logger, at every level, with each throwable's printed stack trace.LicenseKeyMaskTests.maskIn_EncodedKeyShowsThePlainLast4,LicenseKeyMaskTests.maskIn_MasksAKeyEncodedInLowerCaseandDataUpdateServiceLicenseKeyTests.KeyTypedIntoTheUrl_LogsMaskedKeyare written to fail there. In run 42727515077 (branchtmp/red-85) all three failed, as expected.maskIn_EncodedKeyShowsThePlainLast4gave...********************[****]B%3Dwhere...12%2B%3Dwas expected.maskIn_MasksAKeyEncodedInLowerCaseleft the key unmasked,...license=ABCDEFGHIJKLMNOPQRST12%2b%3d.KeyTypedIntoTheUrl_LogsMaskedKeyfound the test key in full in the debug linedownloadFile from https://distributor.example/api/v2/download?LicenseKeys=...and in the error line for the 403.downloadFileback above it, as 4261571 addedlicenseKeysToMaskbetween them.Other drafts that touch the same lines
#79, at its head adce29b, rewrites the logging of
DataUpdateServiceDefault.downloadFile. It names the URL unmasked in its debug line, in the "Checking for update", "Downloaded new data" and "No data newer" info lines, and throughconnectFailure(url, ...)anddownloadFailure(url, ...), and in both catch blocks it copiese.getMessage()into the message text before passing the throwable. Whichever of the two merges second must do four things.licenseKeys = licenseKeysToMask(dataFile, url)andloggedUrl = LicenseKeyMask.maskIn(url, licenseKeys)once at the top ofdownloadFile, and passloggedUrl, noturl, to every message and helper that names the URL. The realurlis used only fornew URI(url.trim()).toURL().Throwable masked = LicenseKeyMask.maskIn(e, licenseKeys)and usemasked.getMessage()for the error detail text as well asmaskedas the logged throwable, because an HTTP library's message names the URL.DataUpdateServiceLicenseKeyTests. It captures every logger call at every level, so it fails for any message that is missed.errortowarn, so those tests' expected level changes on Make the data update follow the specification #79's side. The lines still name the URL throughdownloadFailure(url, ...), so they needloggedUrltoo.The change that discovers when the Cloud Request Engine is built (#86, at its head 7d4cebd) also conflicts with this branch in
CloudRequestEngineDefault.java. So does the change that sends only the Evidence the cloud accepts (#89, at its head 38bccd2), because it is built on #86's branch. Every other file, and the rest of that file, merges without a conflict.getLicenseKeys()there. Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass #86 addssetTimeouts(HttpURLConnection)there and renamesgetCloudPropertiestofetchPublicProperties.getLicenseKeys()andsetTimeouts(HttpURLConnection), and uses Discover when the Cloud Request Engine is built, and retry on first use only after a failure that can pass #86's name for the method,fetchPublicProperties. Send only the Evidence the cloud accepts from the Cloud Request Engine #89 takes the same resolution.Full build
The full build and test matrix passed every job other than macOS on head e3672e5 in run 42773815977 on 10 November 2026, 11 jobs succeeded, run as a dry run so nothing merged. 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.