From eb5e1403612f2e97a72bd65a5e8d3af2aafd7b4d Mon Sep 17 00:00:00 2001 From: Matthew Podwysocki Date: Mon, 14 Sep 2026 13:15:45 -0400 Subject: [PATCH] Teach both installers the telemetry opt-out's new name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The binary's opt-out was renamed from `DISABLE_TELEMETRY` to `MAPBOX_CLI_NO_TELEMETRY` and the break was documented honestly. Both installers were left on the old name, so after that release neither variable silenced both halves: the documented one stopped the CLI's markers but not the installer's, and the old one did the reverse. `docs/distribution.md` in the private repo already claimed the fix — "`MAPBOX_CLI_NO_TELEMETRY` switches all of that off … Both installers carry their own copy of this logic, and both test suites pin it in both directions." The first half was false and the second pinned the wrong name. This makes that documentation true rather than changing it. **The old name keeps working here, and only here.** The binary's break was announced in a changelog someone upgrading can read. An installer is fetched and executed in one line — `curl … | sh`, `irm … | iex` — so its reader has no release notes in front of them, and breaking an opt-out is the one change that must not happen quietly. Asymmetric on purpose, and the comment in each file says so. When both are set the new name wins, including when it declines: an explicit `MAPBOX_CLI_NO_TELEMETRY=0` beats a stale `DISABLE_TELEMETRY=1` left in an image from before the rename. Without that the old variable could never be retired. Verified rather than reasoned about: each script's decision block was run over the cases directly — nine in `sh`, eight in `pwsh` 7.6.6 — covering both names, both precedence directions, the six false spellings, an unknown spelling, whitespace, and a set-but-empty new name as a deliberate clear. Both real harnesses then got cases asserting the new name opts out, wins when the two disagree, and wins in the opt-out direction too. `test-install.sh` reports all cases passed; `test-install.ps1` reports all good. Ported from mapbox/mapbox-cli-private#141, which cannot merge there now that `oss/` is a submodule (mapbox/mapbox-cli-private#132). That PR also edited `docs/distribution.md`, which lives in the private repo and stays there. --- CHANGELOG.md | 9 ++++++++ scripts/install.ps1 | 30 +++++++++++++++++++-------- scripts/install.sh | 45 +++++++++++++++++++++++++++++----------- scripts/test-install.ps1 | 40 +++++++++++++++++++++++++++++++++-- scripts/test-install.sh | 41 +++++++++++++++++++++++++++++++++--- 5 files changed, 139 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9620cf0..a57557b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,15 @@ the Mapbox APIs' own response bodies are not. this release should be able to find it. Both are now covered by tests against RFC 7636, which they were not before. +- Both installers now honour `MAPBOX_CLI_NO_TELEMETRY`, the name the binary + reads. They were left on the old `DISABLE_TELEMETRY` when the binary was + renamed, so neither name silenced both halves: the documented variable + stopped the CLI's markers but not the installer's, and the old one did the + reverse. `DISABLE_TELEMETRY` keeps working **in the installers only** — the + binary's break was announced, and a script fetched and run in one line has + no release notes in front of the reader, so breaking an opt-out there would + have happened silently. When both are set the new name wins. + ## 0.1.8 - 2026-09-14 Initial beta release. The next release is `0.2.0`. diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 8b657b0..0ef900b 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -86,21 +86,33 @@ # because this value comes from the environment and ends up in a header. $InstallSource = $env:MAPBOX_CLI_INSTALL_SOURCE - # The switch src/http.rs honours for the CLI's own User-Agent, read here - # the same way, because someone who put DISABLE_TELEMETRY=1 in a Dockerfile - # and then runs `irm ... | iex` in the same file has already said which way - # they want it. The product token above survives it - the equivalent of + # The switch src/telemetry.rs honours for the CLI's own User-Agent, read + # here the same way, because someone who put it in a Dockerfile and then + # runs `irm ... | iex` in the same file has already said which way they + # want it. The product token above survives it - the equivalent of # `mapbox-cli/` going out either way - and the triple and the # source tag are what it drops. # + # Two names, and only the binary dropped the old one. + # MAPBOX_CLI_NO_TELEMETRY is the documented switch; DISABLE_TELEMETRY is + # what it was called before, and this script still honours it. The rename + # was announced as breaking for the binary; nothing announced it for the + # installers, which are fetched and run in one line with no release notes + # in front of the reader - and breaking an opt-out is the one change that + # must not happen quietly. So both work here, and the new name wins when + # both are set. See mapbox/mapbox-cli-private#140. + # # Unset, empty or whitespace is a cleared variable. `0`, `f`, `false`, `n`, # `no` and `off` are clap's false spellings, the same reading this script - # already gives MAPBOX_NO_MODIFY_PATH, so DISABLE_TELEMETRY=0 is someone - # declining the opt-out rather than taking it. Anything else opts out, - # including a spelling nobody planned for: the safe reading of a value we - # do not know, on a variable by that name, is the one that sends less. + # already gives MAPBOX_NO_MODIFY_PATH, so a `0` is someone declining the + # opt-out rather than taking it. Anything else opts out, including a + # spelling nobody planned for: the safe reading of a value we do not know, + # on a variable by that name, is the one that sends less. $TelemetryAllowed = $true - $disableTelemetry = $env:DISABLE_TELEMETRY + $disableTelemetry = $env:MAPBOX_CLI_NO_TELEMETRY + if ($null -eq $disableTelemetry) { + $disableTelemetry = $env:DISABLE_TELEMETRY + } if ($null -ne $disableTelemetry) { $disableTelemetry = $disableTelemetry.Trim().ToLowerInvariant() $TelemetryAllowed = ( diff --git a/scripts/install.sh b/scripts/install.sh index 90e2319..17627ef 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -62,24 +62,45 @@ USER_AGENT='mapbox-cli-install/1' # header. INSTALL_SOURCE="${MAPBOX_CLI_INSTALL_SOURCE:-}" -# The switch `src/http.rs` honours for the CLI's own User-Agent, read here the -# same way, because someone who put DISABLE_TELEMETRY=1 in a Dockerfile and -# then pipes this script into sh in the same file has already said which way -# they want it. The product token above is what survives it — the equivalent of +# The switch `src/telemetry.rs` honours for the CLI's own User-Agent, read here +# the same way, because someone who put it in a Dockerfile and then pipes this +# script into sh in the same file has already said which way they want it. The +# product token above is what survives it — the equivalent of # `mapbox-cli/` going out either way — and everything appended below # is what it drops. # +# **Two names, and only the binary dropped the old one.** +# `MAPBOX_CLI_NO_TELEMETRY` is the documented switch; `DISABLE_TELEMETRY` is +# what it was called before, and this script still honours it. The rename was +# announced as breaking for the binary, so a `DISABLE_TELEMETRY=1` there +# genuinely stopped working and the changelog says so. Nothing announced it for +# the installers — this file is fetched and run in one line, so a reader has no +# release notes in front of them — and breaking an opt-out is the one change +# that must not happen quietly. So both work here, the new name wins when both +# are set, and the old one keeps working for the Dockerfile the comment above +# describes. See mapbox/mapbox-cli-private#140. +# # Unset, empty, or whitespace: a cleared variable. `0`, `f`, `false`, `n`, `no` -# and `off` are clap's false spellings, so DISABLE_TELEMETRY=0 is someone -# declining the opt-out rather than taking it. Anything else opts out, -# including a spelling nobody planned for: the safe reading of a value we do -# not know, on a variable by that name, is the one that sends less. +# and `off` are clap's false spellings, so a `0` is someone declining the +# opt-out rather than taking it. Anything else opts out, including a spelling +# nobody planned for: the safe reading of a value we do not know, on a variable +# by that name, is the one that sends less. telemetry_allowed() { - # ASCII-only on purpose, the way `to_ascii_lowercase` is in src/http.rs: - # [:upper:]/[:lower:] would bring the locale into a decision about six - # ASCII spellings, and this script sets no LC_ALL. + # Set at all — even to empty, which is how a shell clears one — means the + # new name is the answer. Otherwise fall back, so the two never have to be + # reconciled: an explicit `MAPBOX_CLI_NO_TELEMETRY=0` beats a stale + # `DISABLE_TELEMETRY=1` left in an image from before the rename. + if [ -n "${MAPBOX_CLI_NO_TELEMETRY+set}" ]; then + _telemetry_switch=$MAPBOX_CLI_NO_TELEMETRY + else + _telemetry_switch=${DISABLE_TELEMETRY-} + fi + + # ASCII-only on purpose, the way `to_ascii_lowercase` is in + # src/telemetry.rs: [:upper:]/[:lower:] would bring the locale into a + # decision about six ASCII spellings, and this script sets no LC_ALL. # shellcheck disable=SC2018,SC2019 - case "$(printf '%s' "${DISABLE_TELEMETRY-}" | + case "$(printf '%s' "$_telemetry_switch" | tr 'A-Z' 'a-z' | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')" in '' | 0 | f | false | n | no | off) return 0 ;; diff --git a/scripts/test-install.ps1 b/scripts/test-install.ps1 index 518eb56..3c65437 100644 --- a/scripts/test-install.ps1 +++ b/scripts/test-install.ps1 @@ -386,9 +386,10 @@ function New-CaseEnv([string]$Name) { Clear-Env 'MAPBOX_CLI_AUTH' Clear-Env 'MAPBOX_TILESETS_CLI' Clear-Env 'MAPBOX_CLI_INSTALL_SOURCE' - # A developer with this set in their own shell would otherwise turn every - # marker case into a failure that looks like the marker broke. + # A developer with either of these set in their own shell would otherwise + # turn every marker case into a failure that looks like the marker broke. Clear-Env 'DISABLE_TELEMETRY' + Clear-Env 'MAPBOX_CLI_NO_TELEMETRY' # What install.ps1 reads to decide where it is running. Set explicitly so # the same case means the same thing on Windows and on the machine this is # written on. @@ -513,6 +514,41 @@ try { Clear-Env 'DISABLE_TELEMETRY' Clear-Env 'MAPBOX_CLI_INSTALL_SOURCE' + Start-Case 'MAPBOX_CLI_NO_TELEMETRY is honoured, and outranks the old name' + New-CaseEnv 'telemetry-new-name' + $env:MAPBOX_CLI_INSTALL_SOURCE = 'dockerfile' + # The documented name, which the binary reads and this script did not until + # mapbox/mapbox-cli-private#140. + $env:MAPBOX_CLI_NO_TELEMETRY = '1' + [IO.File]::WriteAllText($RequestLog, '') + Invoke-Installer + Expect-Status 0 'exits 0 - the install is not what is being switched off' + $logged = @([IO.File]::ReadAllLines($RequestLog) | Where-Object { $_ }) + $bare = @($logged | Where-Object { $_ -like "* $InstallerUa" }) + Expect-Equal '2' ([string]$logged.Count) 'still made both requests' + Expect-Equal '2' ([string]$bare.Count) 'each carries the product token and nothing else' + # Both set and disagreeing. The new name is the documented one, so an + # explicit 0 on it beats a DISABLE_TELEMETRY=1 left in an image from before + # the rename - otherwise the old variable could never be retired. + $env:MAPBOX_CLI_NO_TELEMETRY = '0' + $env:DISABLE_TELEMETRY = '1' + [IO.File]::WriteAllText($RequestLog, '') + Invoke-Installer + $logged = @([IO.File]::ReadAllLines($RequestLog) | Where-Object { $_ }) + $marked = @($logged | Where-Object { $_ -like "* $InstallerUa ($Target) src/dockerfile" }) + Expect-Equal '2' ([string]$marked.Count) 'the new name wins when the two disagree' + # And the other direction, so precedence is pinned rather than implied. + $env:MAPBOX_CLI_NO_TELEMETRY = '1' + $env:DISABLE_TELEMETRY = '0' + [IO.File]::WriteAllText($RequestLog, '') + Invoke-Installer + $logged = @([IO.File]::ReadAllLines($RequestLog) | Where-Object { $_ }) + $bare = @($logged | Where-Object { $_ -like "* $InstallerUa" }) + Expect-Equal '2' ([string]$bare.Count) 'and wins in the opt-out direction too' + Clear-Env 'MAPBOX_CLI_NO_TELEMETRY' + Clear-Env 'DISABLE_TELEMETRY' + Clear-Env 'MAPBOX_CLI_INSTALL_SOURCE' + Start-Case 'a checksum that does not match installs nothing' New-CaseEnv 'bad-sha' $env:MAPBOX_CLI_BASE_URL = "$($open.BaseUrl)/bad-sha-channel" diff --git a/scripts/test-install.sh b/scripts/test-install.sh index 2fe6217..bd396f5 100755 --- a/scripts/test-install.sh +++ b/scripts/test-install.sh @@ -531,9 +531,9 @@ new_case_env() { # case-name PATH="${CASE_SHIMS}:${SAFE_PATH}" unset MAPBOX_CLI_VERSION MAPBOX_CLI_AUTH MAPBOX_INSTALL_TILESETS MAPBOX_TILESETS_CLI unset MAPBOX_CLI_INSTALL_SOURCE - # A developer with this set in their own shell would otherwise turn every - # marker case into a failure that looks like the marker broke. - unset DISABLE_TELEMETRY + # A developer with either of these set in their own shell would otherwise + # turn every marker case into a failure that looks like the marker broke. + unset DISABLE_TELEMETRY MAPBOX_CLI_NO_TELEMETRY export MAPBOX_CLI_BASE_URL MAPBOX_INSTALL_DIR PATH } @@ -658,6 +658,41 @@ expect_in_file "$CURL_LOG" "-A ${INSTALLER_UA} (${TARGET}" 'DISABLE_TELEMETRY=0 expect_in_file "$CURL_LOG" ' src/dockerfile' 'and the tag comes back with it' unset MAPBOX_TEST_CURL_LOG DISABLE_TELEMETRY MAPBOX_CLI_INSTALL_SOURCE +start 'MAPBOX_CLI_NO_TELEMETRY is honoured, and outranks the old name' +new_case_env telemetry-new-name +export MAPBOX_INSTALL_TILESETS=no +export MAPBOX_CLI_INSTALL_SOURCE=dockerfile +shim curl-recording curl +CURL_LOG="${CASE_DIR}/curl-args" +export MAPBOX_TEST_CURL_LOG="$CURL_LOG" + +# The documented name, which the binary reads and this script did not until +# mapbox/mapbox-cli-private#140. +: >"$CURL_LOG" +export MAPBOX_CLI_NO_TELEMETRY=1 +run_piped && status=0 || status=$? +expect_status 0 "$status" 'exits 0 — the install is not what is being switched off' +expect_in_file "$CURL_LOG" "-A ${INSTALLER_UA} " 'still names the installer' +expect_not_in_file "$CURL_LOG" "${INSTALLER_UA} (" 'no platform rides behind it' +expect_not_in_file "$CURL_LOG" ' src/' 'and no source tag either' + +# Both set, disagreeing. The new name is the documented one, so an explicit +# `0` on it beats a `DISABLE_TELEMETRY=1` left in an image from before the +# rename — otherwise the old variable could never be retired. +: >"$CURL_LOG" +export MAPBOX_CLI_NO_TELEMETRY=0 +export DISABLE_TELEMETRY=1 +run_piped || true +expect_in_file "$CURL_LOG" "-A ${INSTALLER_UA} (${TARGET}" 'the new name wins when the two disagree' + +# And the other direction, so precedence is pinned rather than implied. +: >"$CURL_LOG" +export MAPBOX_CLI_NO_TELEMETRY=1 +export DISABLE_TELEMETRY=0 +run_piped || true +expect_not_in_file "$CURL_LOG" "${INSTALLER_UA} (" 'and wins in the opt-out direction too' +unset MAPBOX_TEST_CURL_LOG MAPBOX_CLI_NO_TELEMETRY DISABLE_TELEMETRY MAPBOX_CLI_INSTALL_SOURCE + start 'both installers send the same marker' # The one thing about this marker that cannot be checked by watching a request: # install.sh and install.ps1 each carry their own copy, because each is served