From abdce3c4f1b709557926f8d46f47dbd0df20469e Mon Sep 17 00:00:00 2001 From: David Levi Date: Thu, 27 Aug 2026 00:33:28 +0000 Subject: [PATCH] fix: drop the masked key rendering, clearing CodeQL alert #3 Alert #1 closed on the merge of #18. Alert #3 opened against the replacement, at src/components/web3forms.tsx:182, same rule, js/clear-text-logging. This is not a repeat of the same mistake. The previous version printed the key in full. This one printed four leading and four trailing characters. But "fewer characters" is not the same as "no characters", and the alert is pointing at something real. Which field CodeQL could actually taint --------------------------------------- describeKey returned four fields. Only one of them carried the value's content: length a number, from .length not tainted trimmedWhitespace a comparison result not tainted note one of a fixed set of literals, not tainted (implicit selected by branching flow only) masked key.slice(0,4) + key.slice(-4) TAINTED, via slice So the one field the analyser objects to is the one field genuinely carrying key material. That alignment is the useful signal: the tool was right, and the comment claiming the value was "described, never reproduced" was not quite true, because eight characters of it were being reproduced. Removing it costs nothing ------------------------- Every distinction the branch exists to draw comes from the other three fields: empty length 0, trimmedWhitespace false whitespace-padded length 0, trimmedWhitespace TRUE placeholder text not hex-and-dashes truncated hex-and-dashes but not 36 characters Verified across all five cases: each remains distinguishable, and no four character run of a real key appears anywhere in the output. The masked value was only ever an aid to identifying WHICH key arrived, and it could not do that job here anyway: a valid-but-wrong key passes UUID.test and never reaches this branch. Not dismissed ------------- The alert is resolved by removing the flow, not by marking it a false positive. Calling it one would have required arguing that eight characters of a credential are not credential material, which is the kind of claim this repository has already had to walk back twice. Two stale comments describing the removed masking are corrected in the same change, found by grepping rather than by memory. Verified -------- lint, tsc --noEmit and build all clean. --- src/components/web3forms.tsx | 45 ++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/components/web3forms.tsx b/src/components/web3forms.tsx index 46386de..9054aa6 100644 --- a/src/components/web3forms.tsx +++ b/src/components/web3forms.tsx @@ -39,14 +39,12 @@ type Failure = * What a rejected key looked like, without being the key. * * The diagnostic value is in telling apart empty, whitespace-padded, - * placeholder text and truncated. All four are identifiable from length, - * character class and a masked rendering, none of which require printing the - * value itself. + * placeholder text and truncated. All four come from length, a whitespace flag + * and a character-class note. **No characters of the value are carried.** */ type KeyShape = { length: number; trimmedWhitespace: boolean; - masked: string; note: string; }; @@ -72,18 +70,27 @@ const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; * whole, real key included. Narrow, but real, and the comment asserting it was * impossible was worse than no comment. * - * Masking keeps every distinction the diagnostic actually needs: + * An earlier version of this also emitted a masked rendering, four leading and + * four trailing characters. That has been removed. It was the only field + * carrying characters of the value, and therefore the only one CodeQL could + * taint through `String.prototype.slice`, which is what raised + * js/clear-text-logging alert #3 against the supposedly fixed version. * - * empty length 0, nothing was inlined - * whitespace-padded trimmedWhitespace true, the dashboard value has stray - * characters around it + * Removing it costs nothing, because every distinction worth drawing comes + * from the other three fields: + * + * empty length 0 and trimmedWhitespace false + * whitespace-padded length 0 and trimmedWhitespace TRUE * placeholder text not hex-and-dashes, so something rewrote the value * truncated hex-and-dashes but not 36 characters * - * Four leading and four trailing characters are shown only when there are - * enough of them that the middle stays hidden; below that the value is masked - * entirely, since first-four-and-last-four of a nine-character string is - * effectively the whole string. + * The masked value was only ever an aid to identifying *which* key arrived, + * and it could not do that job here: a valid-but-wrong key passes UUID.test + * and never reaches this branch at all. + * + * What survives is genuinely untainted. `length` is a number, `trimmedWhitespace` + * is a comparison result, and `note` is one of a fixed set of string literals + * selected by branching. None of them propagate the value's content. */ function describeKey(raw: string | undefined): KeyShape { const original = raw ?? ""; @@ -105,14 +112,7 @@ function describeKey(raw: string | undefined): KeyShape { note = "not hex and dashes, so this is placeholder text rather than a key"; } - const masked = - length === 0 - ? "(nothing)" - : length <= 12 - ? "•".repeat(length) - : `${key.slice(0, 4)}…${key.slice(-4)}`; - - return { length, trimmedWhitespace, masked, note }; + return { length, trimmedWhitespace, note }; } const inputClass = @@ -181,8 +181,7 @@ function Web3Form({ console.error( `[web3forms] ${keyName ?? "The access key"} reached the browser in an ` + `unusable form: ${shape.length} characters, ${shape.note}` + - `${shape.trimmedWhitespace ? ", with surrounding whitespace trimmed" : ""}. ` + - `Masked: ${shape.masked}`, + `${shape.trimmedWhitespace ? ", with surrounding whitespace trimmed" : ""}.`, ); setFailure({ kind: "malformed", shape }); setStatus("error"); @@ -225,7 +224,7 @@ function Web3Form({ keyName ? ` (Missing ${keyName}.)` : "" }` : failure?.kind === "malformed" - ? `The access key in this build is not usable: ${failure.shape.length} characters, ${failure.shape.note}${failure.shape.trimmedWhitespace ? ", with surrounding whitespace trimmed" : ""} (shown masked as ${failure.shape.masked}). So ${keyName ?? "the variable"} was not exposed to the build intact.` + ? `The access key in this build is not usable: ${failure.shape.length} characters, ${failure.shape.note}${failure.shape.trimmedWhitespace ? ", with surrounding whitespace trimmed" : ""}. So ${keyName ?? "the variable"} was not exposed to the build intact.` : failure?.kind === "unreachable" ? "The form service could not be reached. A privacy or ad blocker will sometimes block it, so it is worth retrying with one paused." : failure?.detail