Repository navigation
Conversation
A command that resolves no token, such as --version, doctor or generate-skills, sent its event with no auth, so a logged-in user's events named their account only when they called an API. Such events now report the token an API command would use, resolved in the same order: --token, then MAPBOX_ACCESS_TOKEN unless --use-login, then the login for --profile. The service arms and this fallback now share auth::resolve_source, so the order cannot drift. The login is read only when an event is sent, without refreshing it or taking its lock. Help and version runs clap answers itself report nothing when their command line chooses a token, since it can't be read.
`report` returns before set_token when the CLI's token serves it, so the fallback named the user's login as the token the run sent. Clear the candidates on that path. Also drop a per-word format! in the argv check and correct the environment candidate's doc.
A query commands search refuses after clap parsed it went through set_unparsed, whose argv scan gave up on --profile; it now reads the token candidates from the parse. logout removed the login before the event read it, so it now records the token it removes, read first.
mattpodwysocki
left a comment
mattpodwysocki
left a comment
There was a problem hiding this comment.
Nice change, and the privacy side holds up well. The account comes from decoding the token locally, the login is only read after the opt-out check, there's no refresh or lock, and the event still goes out with the CLI's own token. The report-with-CLI-token and logout cases are good catches, and backing out of unparsed help runs instead of guessing feels right.
One thing to fix before merging: the "Privacy" section further down the README (around line 696) still says "the kind of token it used and the Mapbox account that token belongs to". Could you add the same note there that you added under "Each run also sends one event", so both spots say what we actually collect?
One question, not blocking: since the schema doesn't change, an event can't tell "used this token" apart from "would have used it". Anyone counting secret-token usage or splitting by auth.source will now pick up --version and config list runs too. Would a small used: false (or similar) under auth be worth adding so the two stay separable? Happy either way if you think the account is all we need from those runs.
Approving with the README fix. Ran cargo test locally on 5a0099a and everything passed.
Commands that use no token (
--version,doctor,generate-skills, ...) send telemetry withoutauth, so a logged-in user's account only shows up on API commands. These events now report the token an API command would use, in the same order (--token, thenMAPBOX_ACCESS_TOKENunless--use-login, then the--profilelogin). The event schema is unchanged.auth::resolve_source.--token,--profileor--use-loginon the command line report nothing, since clap didn't parse those values.auth logoutreports the account it logged out.reportsent with the CLI's own token reports noauth.Verified:
cargo testpasses, with end-to-end tests for each case; reverting each fix fails its test. Not verified: a real event in Athena; Windows (CI only).