Merge pull request #104 from mapbox/telemetry-cli-token-first

Send telemetry with the CLI's own token only

Author: zmofei

CHANGELOG.md

@@ -31,19 +31,25 @@ that may never merge. They are not releases and are not listed here.
   `cli_token_unavailable` and `cli_token_forbidden` (see
   [Errors](docs/commands.md#errors)).
 
-- The CLI's own token, used for telemetry when you have no token of your
-  own, is now fetched from `https://cli.mapbox.com/config/token.json` and
-  kept in `~/.mapbox/cli-token.json` instead of being built into the binary,
+- The CLI's own token, used for telemetry, is now fetched from
+  `https://cli.mapbox.com/config/token.json` and kept in
+  `~/.mapbox/cli-token.json` instead of being built into the binary,
   so it can be rotated without a release. What changes for an existing
   setup:
   - `MAPBOX_CLI_BUNDLED_TOKEN` is no longer read at build time. A build
     from source fetches the token like a release does.
   - Keeping the token creates `~/.mapbox` if it is missing, even on a run
     whose only network use is telemetry.
-  - When an API refuses the token, the Tokens API is asked why. Only a
-    revoked, invalid, expired or malformed token is replaced, at once.
-    Three fetches in a row that find no usable token, or three refusals
-    in a row of a valid one, pause that for an hour.
+  - When an API refuses the token, the Tokens API is asked why and the
+    endpoint asked for a new token at once. A revoked, invalid, expired or
+    malformed token is replaced; a different token served is used from the
+    next run on.
+  - Three failures in a row, each a fetch that finds no usable token or a
+    refusal with no different token served, stop telemetry sending and
+    fetching for an hour. The count is telemetry's own: it never holds
+    `mapbox report` or `MAPBOX_CLI_TOKEN`, and the waits an earlier dev
+    build kept in `cli-token.json` are ignored. A new token ends it, even
+    one `mapbox report` fetched.
 
 - `tilesets query` renders as a numbered list under `-o text` again, as
   documented. It had printed pretty JSON since 0.2.0, when the command moved
@@ -305,13 +311,14 @@ that may never merge. They are not releases and are not listed here.
   file, so with two installs, set it again after using the older one.
 
 - Each run sends one `cli.command` telemetry event to Mapbox, from a
-  background process the command doesn't wait for, with your own token
-  (`--token`, `MAPBOX_ACCESS_TOKEN` or your login) or, when you have none,
-  one built into the CLI. A build from source has no built-in token, so with
-  no token of your own the event is dropped. It never touches stdout, the
-  exit code or how long a command takes, so a script sees no difference; a
-  machine that already has `~/.mapbox` finds a `.telemetry` directory in it.
-  `MAPBOX_CLI_NO_TELEMETRY=1` turns it off.
+  background process the command doesn't wait for. It is only ever sent
+  with the CLI's own public token, never yours (`--token`,
+  `MAPBOX_ACCESS_TOKEN` or your login), since Mapbox Events keeps the token
+  an event was sent with. When the CLI's token can't be used, or the send
+  fails, the event is dropped; it is never sent twice. It never touches
+  stdout, the exit code or how long a command takes, so a script sees no
+  difference; a machine that already has `~/.mapbox` finds a `.telemetry`
+  directory in it. `MAPBOX_CLI_NO_TELEMETRY=1` turns it off.
 
 - `MAPBOX_CLI_EXTRA_QUERY` appends raw query parameters to every request, in
   the same `k1=v1&k2=v2` shape as a URL's own query string — for an API

README.md

@@ -301,11 +301,10 @@ Each run also sends one event to Mapbox:
 It never includes a file path or free text you typed. It carries a random ID
 that is replaced on the first run a day after it was created. It is sent in
 the background, so a command never waits for it, and dropped if it can't be
-delivered. It is sent with your own token (`--token`, `MAPBOX_ACCESS_TOKEN`
-or your login) when you have one, and with the CLI's own public token
-otherwise; Mapbox Events keeps the token an event was sent with, and the
-account it belongs to, alongside the event. With no token of your own and
-no way to fetch the CLI's, the event is dropped.
+delivered. It is only ever sent with the CLI's own public token, never
+yours (`--token`, `MAPBOX_ACCESS_TOKEN` or your login); Mapbox Events keeps
+the token an event was sent with alongside the event. When the CLI's token
+can't be used, the event is dropped, and it is never sent twice.
 `MAPBOX_CLI_NO_TELEMETRY=1` or `mapbox config set telemetry off` turns it off.
 
 ### Diagnostics and settings
@@ -491,22 +490,27 @@ and `--dry-run` show it alongside everything else on the request.
 
 ### The CLI's own requests
 
-The CLI has a token of its own, for requests it makes for itself. Telemetry
-uses your own token whenever you have one: `--token` or
-`MAPBOX_ACCESS_TOKEN`, then your login. Only when you have none does it use
-the CLI's token:
-`MAPBOX_CLI_TOKEN`, or a public token fetched from
-`https://cli.mapbox.com/config/token.json` and kept in
-`~/.mapbox/cli-token.json`. It is replaced only when the Tokens API says it
-is revoked, invalid, expired or malformed, not when one API refuses it. A
-refusal of a valid token, or one the Tokens API cannot explain, counts; the
-third in a row pauses telemetry's sending with it for an hour, as three
-fetches in a row that find no usable token pause fetching. The token is
-restricted to `https://cli.mapbox.com/`, so every request with it carries
-that as its `Referer`. The CLI's token is only used for what
-`src/cli_token.rs` lists for it: telemetry, and `mapbox report`, which is
-the other way around: it prefers the CLI's token to yours and falls back to
-yours while the CLI's can't be used.
+The CLI has a token of its own, for requests it makes for itself.
+Telemetry uses only that token, never the one you run commands with
+(`--token`, `MAPBOX_ACCESS_TOKEN` or a login): it uses a public token
+fetched from `https://cli.mapbox.com/config/token.json` and kept in
+`~/.mapbox/cli-token.json`. `MAPBOX_CLI_TOKEN` is a developer override that
+replaces that token with another CLI token, such as one for staging, and
+is used only when it is a public `pk.` token. With neither usable, the
+event is dropped, and an event that was refused is never sent again. A
+refusal is checked with the Tokens API and the endpoint asked for a new
+token at once: a token that is revoked, invalid, expired or malformed is
+replaced, and a different token served is used from the next run on.
+Three failures in a row, each a fetch that finds no usable token or a
+refusal the endpoint has no different token for, stop telemetry sending
+and fetching for an hour. That count is telemetry's own, and
+`MAPBOX_CLI_TOKEN` is sent whatever it says. A new token ends it, even one
+`mapbox report` fetched. The token is restricted to
+`https://cli.mapbox.com/`, so every request with it carries that as its
+`Referer`. The CLI's token is only used for what
+`src/cli_token.rs` lists for it: telemetry, and `mapbox report`, which
+prefers the CLI's token and falls back to yours while the CLI's can't be
+used.
 
 ### Proxies
 
@@ -663,7 +667,7 @@ tools are used and how to improve our products.
   `--dry-run` or whether a named profile was used), how it ended and how
   long it took, how many requests it made with their HTTP status and sizes,
   the kind of token it used and the Mapbox account that token belongs to,
-  the Mapbox access token the event itself was sent with, a random
+  the CLI's own public token the event itself was sent with, a random
   identifier that is replaced every day, which run started this one when it
   ran inside another, CLI version, how the CLI was installed, the version
   it was updated from and any newer one it offered, OS/architecture,
@@ -679,8 +683,8 @@ tools are used and how to improve our products.
   adoption, prioritize investments, and improve the reliability,
   performance, and developer experience of our CLIs.
 - **What We Do Not Collect:** Code completion outputs, source code, project
-  file names, directory contents, non-Mapbox API keys, or credentials other
-  than the Mapbox access token an event is sent with.
+  file names, directory contents, non-Mapbox API keys, or credentials,
+  your Mapbox access token included.
 - **Who has Access:** Telemetry data will not be disclosed to, or accessed
   by, third parties other than Mapbox affiliates and passive cloud storage
   and hosting providers necessary to maintain our infrastructure.

docs/architecture.md

@@ -191,7 +191,7 @@ flowchart LR
         events["Mapbox Events<br/>POST /events/v2"]
     end
 
-    bin -- "① spawn detached, event + token<br/>on stdin (never argv or env)" --> child
+    bin -- "① spawn detached, event<br/>on stdin (never argv or env)" --> child
     child -- "② 🔒 HTTPS POST" --> events
 ```
 
@@ -203,15 +203,18 @@ entirely, same as Diagram 2.
 
 Step ① is detached and non-blocking: the parent spawns a copy of itself in
 this special mode and exits without waiting on it, so a dead network or a
-slow endpoint never adds to a command's run time. The event and the token it
-travels with go over the child's stdin rather than its argv or environment,
-for the same `ps`-visibility reason Diagram 3 gives.
-
-Step ②'s token follows `src/cli_token.rs`'s fallback order: the user's own
-token first (flag, environment, or stored login), and only when none of
-those exist, the CLI's `pk.` token as a last resort so usage from a user who
-has never logged in is still counted. Every build fetches that token from
-`https://cli.mapbox.com/config/token.json` and keeps it in
+slow endpoint never adds to a command's run time. The event goes over the
+child's stdin rather than its argv or environment, for the same
+`ps`-visibility reason Diagram 3 gives. The child is not handed the user's
+token at all.
+
+Step ② is sent with the CLI's own `pk.` token only, never the user's
+(`src/cli_token.rs`): `MAPBOX_CLI_TOKEN` when set to a `pk.` token,
+otherwise the one every build fetches from
+`https://cli.mapbox.com/config/token.json` and keeps in
 `~/.mapbox/cli-token.json` until the Tokens API says it is dead
-(`src/cli_token/fetch.rs`). With no login and no way to fetch it, the event
-is simply dropped.
+(`src/cli_token/fetch.rs`). With no usable CLI token the event is dropped,
+and a send that fails is not tried again. Three failures in a row (a fetch
+with no usable token, or a refusal with no different token served) quiet
+telemetry for an hour, a count kept per caller in that same file. A new
+token saved by any caller ends every caller's count.

src/auth.rs

@@ -479,8 +479,7 @@ fn credentials_path_readonly(profile: Option<&str>) -> Option<PathBuf> {
 /// directory as a side effect of reading it — see
 /// [`credentials_path_readonly`]. What [`profiles`] reads each stored
 /// profile through, since listing what exists must not be the reason a
-/// directory starts to exist or its permissions change, and what
-/// [`crate::cli_token`] reads the login through for the same reason.
+/// directory starts to exist or its permissions change.
 pub(crate) fn load_credentials_readonly(profile: Option<&str>) -> Option<Credentials> {
     let data = std::fs::read_to_string(credentials_path_readonly(profile)?).ok()?;
     serde_json::from_str(&data).ok()
@@ -718,7 +717,7 @@ pub fn warn_if_environment_token_shadows_login(profile: Option<&str>, remedy: &s
     }
 }
 
-pub(crate) fn token_needs_refresh(token: &str) -> bool {
+fn token_needs_refresh(token: &str) -> bool {
     match token_expires_at(token) {
         Some(exp) => {
             let now = std::time::SystemTime::now()
@@ -1068,8 +1067,8 @@ pub fn logout(profile: Option<&str>, mode: Mode) -> Result<()> {
     let path = credentials_path(profile)?;
     let had_credentials = path.exists();
     if had_credentials {
-        // Waits out a refresh in flight, such as the telemetry sender's from
-        // the previous run, which would otherwise write the login back.
+        // Wait for another run's login refresh to finish, so it can't write
+        // the login back after it is removed.
         let _lock = CredentialLock::acquire(profile).ok();
         crate::run_record::set_auth_step("remove_credentials");
         std::fs::remove_file(&path)?;