feat(telemetry): one-time opt-out beacon + banner Settings deep-link (MCP-2482)#684
Merged
Merged
Conversation
Deploying mcpproxy-docs with
|
| Latest commit: |
c86024b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a71f377b.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://feat-mcp-2482-telemetry-opto.mcpproxy-docs.pages.dev |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27548057538 --repo smart-mcp-proxy/mcpproxy-go
|
| } | ||
| req.Header.Set("Content-Type", "application/json") | ||
|
|
||
| resp, err := s.client.Do(req) |
…(MCP-2482) Part B — server-side opt-out beacon: - Add internal/telemetry/optout.go: TelemetryDisableTransition (resolved-state compare, nil == enabled per MCP-2477), SendOptOutBeacon (reuses the existing /heartbeat ingest with event:"telemetry_disabled" + only the anonymous_id, no usage payload), and Service.NotifyConfigChanged. - Detect the enabled->disabled flip on config write and fire EXACTLY ONE best-effort, fire-and-forget beacon; latch optedOut so no further heartbeats are emitted. Re-enable clears the latch (exactly once per flip). - Wire NotifyConfigChanged into the daemon's REST apply path (runtime.ApplyConfig) and disk-reload path (ReloadConfiguration) so web UI + macOS app are covered, and into `mcpproxy telemetry disable` so the CLI path is covered (no fsnotify watcher means the daemon won't auto-reload the file). Send failure still disables; disabled->disabled / env-disabled emit nothing. Part A — banner deep-link + disclosure: - TelemetryBanner: add "Manage in Settings" link to /settings?focus=telemetry.enabled and a transparency note about the one-time opt-out signal. - Settings.vue: focusField() resolves a field key to its tab, opens the enclosing accordion, scrolls to and highlights the row. - Disclosure line added near the telemetry toggle (field help + confirm copy). Docs: document the one-time opt-out signal in docs/features/telemetry.md. Related #MCP-2482
…deQL SSRF) CodeQL go/request-forgery (CWE-918) flagged optout.go because the destination URL flowed as a function parameter directly from the config source to the http.Do sink. Make SendOptOutBeacon a *Service method that reads the resolved s.endpoint/s.config — the same struct-field indirection the existing heartbeat and feedback senders use (which CodeQL does not flag) — so the beacon can never target an arbitrary caller-supplied URL. The CLI path now constructs a telemetry.Service and calls the method instead of passing a raw URL. No behavior change: still exactly one fire-and-forget beacon to /heartbeat on the enabled->disabled flip. Related #MCP-2482
…E-918 Apply the repo's established request-forgery barrier (mirror validateRegistryURL): parse the configured telemetry endpoint, constrain the scheme to http/https, require a host, and issue the beacon against the re-serialized URL. Rejects file://, gopher://, and schemeless/malformed endpoints before any request. Related #MCP-2482
…ist guard) Scheme/host-presence validation alone did not satisfy CodeQL go/request-forgery; the established repo barrier (validateRegistryURL) is a host equality guard. Pin the beacon destination to the built-in telemetry host or a loopback address (tests/local dev) and reject anything else before issuing the request. This both clears the alert and genuinely hardens the path — the anonymous ID can only ever reach the official endpoint or localhost, never an arbitrary host from a malformed/hostile telemetry.endpoint value (documented as a testing override). Related #MCP-2482
Dumbris
force-pushed
the
feat/mcp-2482-telemetry-optout-beacon
branch
from
68ea2e4 to
59d6ea3
Compare
Move the host allowlist check inline into validateTelemetryURL as an
'if !match { return err }' equality guard on the same control-flow path as the
request sink, mirroring validateRegistryURL (the repo's CodeQL-accepted
request-forgery barrier). The prior bool-returning helper put the guard across a
function boundary that CodeQL's barrier-guard analysis did not propagate.
Related #MCP-2482
…918) Earlier barrier shapes (helper returning bool; EqualFold + net.ParseIP/IsLoopback) did not satisfy CodeQL go/request-forgery. Switch to an inline exact-equality allowlist (switch on the lower-cased host against the built-in telemetry host and explicit loopback literals) — the canonical barrier shape the scanner recognizes, on the same control-flow path as the request. Behaviour unchanged: prod host and loopback (tests/local) allowed, everything else rejected. Related #MCP-2482
Member
Author
|
codex review (gpt-5.5, in-checkout) → REQUEST_CHANGES. Beacon payload shape and the banner deep-link + disclosure are good. Three blocking issues (2× P1 privacy-relevant):
|
…e, CLI guards (MCP-2482) Codex REQUEST_CHANGES on PR #684: 1. [P1] Respect env-resolved disabled state. The transition gated only on Config.IsTelemetryEnabled() (MCPPROXY_TELEMETRY), so DO_NOT_TRACK=1 / CI=true could emit a beacon when telemetry was never enabled. Add EffectiveTelemetryEnabled() = IsTelemetryEnabled() && !IsDisabledByEnv(), and use it for the transition, the New() resolvedEnabled seed, and the CLI. 2. [P1] Heartbeat-after-opt-out race. sendHeartbeat checked optedOut only at entry; a heartbeat in flight when the latch flipped still shipped a full usage payload. Re-check the latch immediately before transmit so no usage data leaves after opt-out. 3. [P2] CLI disable path. Routed through a single guarded entry point (Service.EmitOptOutBeacon — applies semver/dev, env, and anon-id guards and owns the send) instead of calling SendOptOutBeacon directly (which bypassed the guards). The CLI now confirms "Telemetry disabled." before the best-effort beacon (non-blocking) with a short 3s timeout. Tests: env-disabled emits nothing; mid-flight opt-out suppresses transmit; EmitOptOutBeacon guard matrix (dev/env/no-anon-id skip, eligible sends). CLI smoke: enabled=1 beacon, DO_NOT_TRACK/CI=0. Related #MCP-2482
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code review ACCEPT (codex out → Claude-reviewer fallback). Round-2 fixes all verified w/ regression tests: (1) EffectiveTelemetryEnabled gates on IsDisabledByEnv (DO_NOT_TRACK/CI) so no beacon when telemetry never truly enabled; (2) sendHeartbeat re-checks optedOut immediately before client.Do -> no usage payload after opt-out; (3) CLI disable routes through guarded EmitOptOutBeacon (semver/env/anon-id guards). Beacon payload minimal, exactly-once, best-effort. go test ./internal/telemetry/... passes.
Conflict from #685 (instructions prefill) + #681 (telemetry serialization) landing on main while this branch was in review. Resolved Settings.vue: - imports: union of watch (prefill) + nextTick/useRoute (banner focus deep-link) - onMounted: keep #685's watch(defaultInstructions) prefill trigger, use this branch's async onMounted for the focus query-param handling, drop the duplicate loadConfig() (body already awaits loadConfig()). Verified: 14 settings unit tests pass (instructions-prefill + prefill-dirty). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dumbris
added a commit
that referenced
this pull request
Jun 15, 2026
The three rapid-based property tests (TestRapidQuarantineStateMachine, TestRapidInvariant_ChangedNeverAutoApproved, TestRapidInvariant_PendingNeverAutoApproved) each create and tear down hundreds of BBolt-backed Runtimes per rapid.Check run. Under -race they take several minutes; on Windows the slower file IO/timers push them past the 5m package timeout of the Unit Tests (windows-latest) job, which runs 'go test -race -timeout 5m ./...' without -short. This produced flaky 'panic: test timed out after 5m0s' reds in internal/runtime unrelated to any PR change (#675, #685, #684). The existing testing.Short() guards never fire because the job omits -short. Add a documented Windows skip (mirroring apply_config_restart_test.go) to the offending tests only. The invariants they assert are platform-independent, so Linux/macOS coverage plus the dedicated heavy-runtime CI job is sufficient. The global timeout is left untouched. Related #MCP-2493
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements MCP-2482 — a telemetry consent banner Settings deep-link (Part A) and a one-time, server-side opt-out beacon (Part B). Reuses the MCP-2477 resolved-state convention (
IsTelemetryEnabled—nilmeans enabled).Part B — server-side opt-out beacon (
internal/telemetry)TelemetryDisableTransition(prior, next)— detects the enabled→disabled flip by comparing resolved telemetry state.SendOptOutBeacon— posts{"event":"telemetry_disabled","anonymous_id":"…"}to the existing/heartbeatingest (no new endpoint). Carries only the anonymous install ID — no usage payload. Runs through the sameScanForPIIguard.Service.NotifyConfigChanged(newCfg)— on a flip: latchesoptedOut(stops all further heartbeats) then fires exactly one best-effort, fire-and-forget beacon (5s timeout). A send failure still disables.disabled→disabled/ reload-while-disabled / env-disabled emit nothing. Re-enable clears the latch (exactly once per flip).runtime.ApplyConfig(REST → web UI + macOS app),runtime.ReloadConfiguration(disk reload), andmcpproxy telemetry disable(CLI — there is no fsnotify watcher, so the CLI fires its own beacon).Part A — banner deep-link + disclosure (Vue)
TelemetryBannergains a "Manage in Settings" link →/settings?focus=telemetry.enabled, plus a transparency note.Settings.vuefocusField()resolves a field key to its tab, opens the enclosing accordion, scrolls to + highlights the row.Tests (TDD)
internal/telemetry/optout_test.go: transition table; beacon payload shape (path/method, event+anon ID, asserts no usage fields); fires exactly once on disable; nothing when already disabled; send failure still disables; no further heartbeats after opt-out.Verification
go build ./...+-tags server✅ ·go test ./internal/telemetry ./internal/runtime ./cmd/mcpproxy -race✅golangci-lint v2.5.0(CI config) → 0 issues ·gofmtclean ·vue-tsc --noEmitclean ·make build✅scripts/test-api-e2e.sh→ 65/65 ✅telemetry disableagainst a capture server → exactly 1 beacon{"event":"telemetry_disabled","anonymous_id":"smoke-anon-001"}to/v1/heartbeat; seconddisable→ 0.Docs
docs/features/telemetry.mddocuments the one-time opt-out signal.Related #MCP-2482