Conversation
An explicit telemetry = true in bunfig.toml sets TriState::Yes, and is_enabled() now returns true for that state. The environment variable only decides the Unknown state, as before #36165. Add crash_handler.isAnalyticsEnabled() to bun:internal-for-testing so a test can read the value. Debug builds never consult it in the crash reporter.
|
Warning Review limit reached
On-demand reviews are free for the next 23 days. After that, they cost $0.25 per reviewed file. Or wait 9 minutes for your next included review. View limit detailsLimit details: You’ve used all 5 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
| match enabled() { | ||
| TriState::Yes => !env_var::DO_NOT_TRACK.get().unwrap_or(false), | ||
| TriState::Yes => true, |
There was a problem hiding this comment.
🔴 Project-controlled bunfig.toml telemetry = true now silently overrides a user's system-wide DO_NOT_TRACK=1 privacy opt-out
Extended reasoning...
bunfig.toml is a project-level file that is routinely checked into repositories a user clones — it is not necessarily authored by the user running Bun. After this change, TriState::Yes => true returns unconditionally, so a repo that ships telemetry = true in its bunfig defeats the user's DO_NOT_TRACK=1 environment variable, and is_reporting_enabled() at src/crash_handler/lib.rs:2729 proceeds to upload crash reports. Relative to the base branch (where TriState::Yes still consulted DO_NOT_TRACK and returned false), a user who has opted out of tracking system-wide now has crash reports uploaded to bun.report because of a file in a project they cloned, without any visible indication. The PR frames this as restoring pre-#36165 Zig behavior, but the docs update and PR body do not address that bunfig is project-scoped rather than user-scoped — this inverts the usual precedence where a user-level privacy signal wins over project config. If the intent is that only a user's own explicit opt-in should override DO_NOT_TRACK, the override belongs in the global bunfig…
Verification: normal — The scenario is real and reachable, and merging leaves the base branch worse on a privacy-relevant path. - src/analytics/lib.rs:51 now reads TriState::Yes => true; the base had TriState::Yes => !env_var::DO_NOT_TRACK.get().unwrap_or(false). So with DO_NOT_TRACK=1 set in the user's environment, is_enabled() flips from false (base) to true (PR) whenever bunfig set Yes. -…
There was a problem hiding this comment.
The scenario is reachable. telemetry is parsed only for run commands, and run commands read the local bunfig.toml only (the global file is loaded for package manager commands). So a telemetry = true in a cloned repo is the only way this arm is set, and with this PR it wins over DO_NOT_TRACK=1.
Both orders are defensible. The PR restores the order the code had before #36165, which is what the maintainer who reported the bug asked for. The other order (the environment variable always wins) keeps the current main behavior and needs a docs change instead of a code change. I have raised the choice with the maintainer. This thread stays open until that decision lands.
|
Updated 12:35 AM PT - Aug 28th, 2026
❌ @robobun, your commit 37aec1d has 3 failures in
🧪 To try this PR locally: bunx bun-pr 40728That installs a local version of the PR into your bun-40728 --bun |
|
CI status for 37aec1d (build 107449): 178 of 181 jobs passed. The 3 failed jobs do not touch this diff, and all 3 fail on main too:
|
Problem
telemetry = trueinbunfig.tomlno longer enables crash reports whenDO_NOT_TRACK=1is set. The explicit opt-in is ignored.is_enabled()insrc/analytics/lib.rs:49returns!DO_NOT_TRACKforTriState::Yes. Only the bunfig setter (src/bunfig/bunfig.rs:394) setsYes. TheUnknownarm already consultsDO_NOT_TRACK, soYesdoes not need to.trueto!DO_NOT_TRACKwithout a note in the PR body and without a test. The Zig source had.yes => true.Fix
TriState::Yes => true. An explicit bunfig value wins over the environment.DO_NOT_TRACKdecides only theUnknownstate. The bunfig docs now state this order.is_reporting_enabled()before they reachis_enabled(). So a test cannot observe the value through a crash. This PR addscrash_handler.isAnalyticsEnabled()tobun:internal-for-testing, next togetFeatureData().test/config/bunfig/telemetry.test.ts(4 cases). With the binding but without the one-line fix, thetelemetry = trueplusDO_NOT_TRACK=1case printsfalse. Also rantest/config/bunfig/bunfig-errors.test.ts,test/cli/run/run-crash-handler.test.ts,test/internal/macos-cross-config.test.ts, andtest/internal/source-lints/.Background
bun_analytics::ENABLEDis a process-globalTriState(Yes,No,Unknown). It starts asUnknown. The bunfig loader setsYesorNofrom thetelemetrykey.is_enabled()resolvesUnknownonce from the environment (DO_NOT_TRACK,HYPERFINE_RANDOMIZED_ENVIRONMENT_OFFSET) and caches the result.src/crash_handler/lib.rs:2729) is the only reader. It checksBUN_CRASH_REPORT_URL,BUN_ENABLE_CRASH_REPORTING, debug, and ASAN first, thenis_enabled().Notes
src/analytics/lib.rs. The PR body lists no telemetry change. Rust unit tests for the workspace do not run in CI, and no JS test covered this path.DO_NOT_TRACKandHYPERFINE_RANDOMIZED_ENVIRONMENT_OFFSETfrom the inherited env before it sets its own values, becausebunEnvspreads the agent environment.bun -eruns as the auto command and loadsbunfig.tomlfrom the cwd, so the test uses-ewith atempDirthat holds the bunfig.cargo clippy -p bun_analytics --no-depsis clean.cargo fmt --checkand prettier are clean for the changed files.[review] gate passed · iteration 0 · 5 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file