Opt-in crash reporting (Sentry) behind two gates that both have to hold

.github/workflows/release.yml

@@ -185,11 +185,15 @@ jobs:
       # checkout; windows-msvc-ci.yml A/B-validates this same path on every PR.
       - name: Build satellite.exe (MSVC + vcpkg, hardened)
         shell: pwsh
+        env:
+          # Empty for anything but a release from this repo: secrets are not
+          # exposed to forks, so a fork build carries no DSN and cannot report.
+          SATELLITE_SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
         run: |
           $env:VCPKG_ROOT = $env:VCPKG_INSTALLATION_ROOT
           $generator = ./scripts/windows-vs-generator.ps1
           $disp = ("${{ inputs.tag || github.ref_name }}") -replace '^v',''
-          cmake --preset windows-msvc -G $generator -DSATELLITE_RELEASE_VERSION="$disp"
+          cmake --preset windows-msvc -G $generator -DSATELLITE_RELEASE_VERSION="$disp" -DSATELLITE_SENTRY_DSN="$env:SATELLITE_SENTRY_DSN"
           cmake --build --preset windows-msvc --target satellite
           if (-not (Test-Path satellite.exe)) { throw "MSVC build did not produce satellite.exe at repo root" }
 
@@ -285,9 +289,13 @@ jobs:
         run: brew install cmake pkg-config libsodium opus
 
       - name: Build satellite.app (stub)
+        env:
+          # Empty for anything but a release from this repo: secrets are not
+          # exposed to forks, so a fork build carries no DSN and cannot report.
+          SATELLITE_SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
         run: |
           tag="${{ inputs.tag || github.ref_name }}"
-          cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DSATELLITE_RELEASE_VERSION="${tag#v}"
+          cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DSATELLITE_RELEASE_VERSION="${tag#v}" -DSATELLITE_SENTRY_DSN="${SATELLITE_SENTRY_DSN}"
           cmake --build build --config Release --target satellite -j "$(sysctl -n hw.ncpu)"
 
       - name: Sign + notarize (if Developer ID configured)
@@ -407,9 +415,13 @@ jobs:
           ref: ${{ inputs.tag || github.ref }}
 
       - name: Build satellite
+        env:
+          # Empty for anything but a release from this repo: secrets are not
+          # exposed to forks, so a fork build carries no DSN and cannot report.
+          SATELLITE_SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
         run: |
           tag="${{ inputs.tag || github.ref_name }}"
-          cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DSATELLITE_RELEASE_VERSION="${tag#v}"
+          cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DSATELLITE_RELEASE_VERSION="${tag#v}" -DSATELLITE_SENTRY_DSN="${SATELLITE_SENTRY_DSN}"
           cmake --build build --config Release --target satellite -j "$(nproc)"
 
       - name: Package (.deb via CPack)
@@ -537,9 +549,13 @@ jobs:
           ref: ${{ inputs.tag || github.ref }}
 
       - name: Build satellite
+        env:
+          # Empty for anything but a release from this repo: secrets are not
+          # exposed to forks, so a fork build carries no DSN and cannot report.
+          SATELLITE_SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
         run: |
           tag="${{ inputs.tag || github.ref_name }}"
-          cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DSATELLITE_RELEASE_VERSION="${tag#v}"
+          cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DSATELLITE_RELEASE_VERSION="${tag#v}" -DSATELLITE_SENTRY_DSN="${SATELLITE_SENTRY_DSN}"
           cmake --build build --config Release --target satellite -j "$(nproc)"
 
       - name: Package (.rpm via CPack)
@@ -644,6 +660,10 @@ jobs:
       # copy) lives in scripts/build-appimage.sh so a local AppImage and the
       # released one come off the same path (the dish-linux model).
       - name: Build AppImage
+        env:
+          # Empty for anything but a release from this repo: secrets are not
+          # exposed to forks, so a fork build carries no DSN and cannot report.
+          SATELLITE_SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
         run: |
           tag="${{ inputs.tag || github.ref_name }}"
           SATELLITE_VERSION="${tag#v}" bash scripts/build-appimage.sh

CHANGELOG.md

@@ -249,6 +249,44 @@ MINGW64; the Windows lane now also carries warnings-as-errors like Linux and
 macOS; `vcpkg.json` sat at 1.0.0 while `/VERSION` said 1.1.0 (now checked by
 version-consistency.yml).
 
+Opt-in crash reporting. Satellite has never transmitted anything, and on
+Linux and macOS it had no crash recorder at all: a segfault died with whatever
+the distro's core-dump collector happened to catch. Windows wrote a local
+minidump nobody was told about. This adds Sentry behind two independent gates,
+because one is not enough. The operator's switch (Settings, Diagnostics) is
+off by default and stays off through an upgrade: an install that never saw the
+ask must not start transmitting on its owner's behalf, which is why the config
+default is `false` rather than matching the Dish clients' default-on. Turning
+it off disarms the SDK immediately rather than at the next restart, since
+withdrawing consent has to stop the next crash and not the one after it.
+
+The second gate is the build. `SATELLITE_SENTRY_DSN` is empty in CMake and is
+only ever filled in by `release.yml` from a repository secret, so a local
+build, a PR build and a build from a fork (secrets are not exposed to forks)
+carry no DSN and cannot transmit no matter what the switch says. The Sentry
+environment is derived from `SATELLITE_RELEASE_VERSION`, which only the release
+workflow sets, so `production` is not a label a developer build reaches by
+accident -- and because it is only a label (`scripts/build-appimage.sh` sets
+that variable when run by hand), the DSN is the gate that actually holds.
+The release string uses the display version, so a `-dev` build cannot file
+itself against a real release and mix unsymbolicated frames into genuine data.
+`$SENTRY_DSN` still works as a developer escape hatch, and still respects the
+opt-in.
+
+Nothing about the local artifacts changes. On Windows the existing
+`dumps\*.dmp` writer keeps running, and `dumpFilter` now chains to whatever
+top-level filter was installed before it instead of swallowing the exception,
+so the local dump and the Sentry report both see the crash rather than
+whichever recorder armed last winning outright. Automatic session tracking is
+turned off, because it defaults to on and a server meant to run unattended for
+weeks should not report every start and stop to anyone; the crash is the
+payload. PII is off too, but by not touching it: sentry-native does not send it
+by default, and the setter that would change that exists only on Nintendo
+Switch.
+The status payload reports the opt-in and whether it actually armed as separate
+fields, so a build with no DSN says so instead of claiming reports are going
+somewhere they are not.
+
 ## 1.1.0
 
 No protocol changes. Distribution release: every shipping platform now also

CMakeLists.txt

@@ -38,6 +38,46 @@ if(WIN32)
     add_compile_definitions(NOMINMAX)
 endif()
 
+# ── Crash reporting (Sentry) ──────────────────────────────────────────────
+#
+# Two independent gates, on purpose.
+#
+# The ENVIRONMENT is a label. It is derived from SATELLITE_RELEASE_VERSION,
+# which only .github/workflows/release.yml sets, so "production" is not a name
+# a developer build reaches by accident. It is still only a label: anyone can
+# pass -DSATELLITE_RELEASE_VERSION, and scripts/build-appimage.sh does exactly
+# that when run by hand.
+#
+# The DSN is the real gate. It is empty here and is only ever filled in by
+# release.yml from the SENTRY_DSN repository secret, so a local build, a PR
+# build and a build from a fork (secrets are not exposed to forks) carry no
+# DSN and physically cannot transmit, whatever label they wear. Labels drift;
+# a missing DSN cannot.
+#
+# RELEASE uses the DISPLAY version, not SATELLITE_VERSION: the latter is
+# stripped to bare MAJOR.MINOR.PATCH above, so a dev build would file itself
+# against the real release and mix unsymbolicated frames into genuine data.
+# DISPLAY carries the "-dev" suffix that keeps the two apart.
+if(DEFINED SATELLITE_RELEASE_VERSION AND NOT "${SATELLITE_RELEASE_VERSION}" STREQUAL "")
+    set(SATELLITE_SENTRY_ENVIRONMENT "production")
+else()
+    set(SATELLITE_SENTRY_ENVIRONMENT "development")
+endif()
+
+set(SATELLITE_SENTRY_DSN "" CACHE STRING
+    "Sentry DSN. Empty (the default) disables crash reporting at compile time.")
+
+# Optional dependency, exactly like the tray: absent means the facade in
+# core/crash_reporting.cpp compiles to inert stubs and everything else still
+# builds. Keeps a source checkout buildable with no extra packages.
+find_package(sentry CONFIG QUIET)
+if(sentry_FOUND)
+    message(STATUS "satellite: Sentry SDK found; crash reporting compiled in "
+                   "(environment: ${SATELLITE_SENTRY_ENVIRONMENT})")
+else()
+    message(STATUS "satellite: Sentry SDK not found; crash reporting stubbed out")
+endif()
+
 # Vendored single-header libraries (nlohmann/json, cpp-httplib) under lib/.
 # SYSTEM keeps their internal warnings out of -Wall/-Wextra//W4, and every
 # target (including pure-core tests) can include them.
@@ -51,6 +91,7 @@ set(SATELLITE_CORE_SOURCES
     src/core/update_service.cpp
     src/core/github_release.cpp
     src/core/network_info.cpp
+    src/core/crash_reporting.cpp
 )
 
 # Currently-shared infrastructure (to be ported to POSIX in Stage 3).
@@ -78,6 +119,7 @@ set(SATELLITE_NET_SOURCES
 set(SATELLITE_ADAPTER_SOURCES
     src/adapters/log_adapter.cpp
     src/adapters/client_adapter.cpp
+    src/adapters/crash_adapter.cpp
     src/adapters/audio/opus_codec.cpp
 )
 
@@ -721,6 +763,18 @@ if(TARGET satellite)
     target_compile_definitions(satellite PRIVATE
         SATELLITE_VERSION_RESOLVED="${SATELLITE_VERSION_DISPLAY}")
 
+    # Crash reporting identity. Only the shipped binary carries these; the test
+    # targets compile core/crash_reporting.cpp against the header's fallbacks,
+    # which resolve to an empty DSN and so can never transmit from a test run.
+    target_compile_definitions(satellite PRIVATE
+        SATELLITE_SENTRY_DSN="${SATELLITE_SENTRY_DSN}"
+        SATELLITE_SENTRY_ENVIRONMENT="${SATELLITE_SENTRY_ENVIRONMENT}"
+        SATELLITE_SENTRY_RELEASE="satellite@${SATELLITE_VERSION_DISPLAY}")
+    if(sentry_FOUND)
+        target_compile_definitions(satellite PRIVATE SATELLITE_HAS_SENTRY=1)
+        target_link_libraries(satellite PRIVATE sentry::sentry)
+    endif()
+
     include(CheckIPOSupported)
     check_ipo_supported(RESULT SATELLITE_IPO_OK OUTPUT SATELLITE_IPO_MSG LANGUAGES CXX)
     if(SATELLITE_IPO_OK)
@@ -881,6 +935,8 @@ satellite_add_pure_test(test_github_release tests/test_github_release.cpp src/co
 satellite_add_pure_test(test_update_service tests/test_update_service.cpp src/core/update_service.cpp)
 satellite_add_pure_test(test_json tests/test_json.cpp)
 satellite_add_pure_test(test_config_json tests/test_config_json.cpp)
+satellite_add_pure_test(test_crash_reporting tests/test_crash_reporting.cpp
+                        src/core/crash_reporting.cpp src/adapters/crash_adapter.cpp)
 satellite_add_pure_test(test_network_info tests/test_network_info.cpp src/core/network_info.cpp)
 satellite_add_pure_test(test_origin_guard tests/test_origin_guard.cpp)
 satellite_add_pure_test(test_status_json tests/test_status_json.cpp)
@@ -975,6 +1031,11 @@ if(APPLE OR CMAKE_SYSTEM_NAME STREQUAL "Linux")
         src/net/local_iface.cpp
         src/core/config_io.cpp
         ${SATELLITE_CORE_SOURCES}
+        # routes_admin.cpp reads crash::active() and flips crash::setEnabled(),
+        # so the route tests need the adapter as well as the pure policy that
+        # rides in SATELLITE_CORE_SOURCES. It compiles inert here: the test
+        # targets never define SATELLITE_HAS_SENTRY.
+        src/adapters/crash_adapter.cpp
     )
     if(APPLE)
         set(SATELLITE_ROUTE_TEST_PLATFORM_SOURCES

scripts/build-appimage.sh

@@ -37,7 +37,10 @@ appdir="${tmp_dir}/Satellite.AppDir"
 
 echo "==> satellite ${version} AppImage (x86_64)"
 
-cmake -S . -B "${build_dir}" -DCMAKE_BUILD_TYPE=Release -DSATELLITE_RELEASE_VERSION="${version}"
+# SATELLITE_SENTRY_DSN is empty unless release.yml exported it from the
+# repository secret, so a hand-run of this script still produces a build
+# that cannot transmit, even though it stamps a release version.
+cmake -S . -B "${build_dir}" -DCMAKE_BUILD_TYPE=Release -DSATELLITE_RELEASE_VERSION="${version}" -DSATELLITE_SENTRY_DSN="${SATELLITE_SENTRY_DSN:-}"
 cmake --build "${build_dir}" --config Release --target satellite -j "$(nproc)"
 
 wget -q https://github.com/linuxdeploy/linuxdeploy/releases/download/continuous/linuxdeploy-x86_64.AppImage -O linuxdeploy

src/adapters/crash_adapter.cpp

@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: LGPL-3.0-or-later
+
+#include "adapters/crash_adapter.h"
+
+#include "core/crash_reporting.h"
+
+#ifdef SATELLITE_HAS_SENTRY
+#include <sentry.h>
+#endif
+
+namespace satellite::crash {
+
+namespace {
+bool g_active = false;
+// Remembered from init() so a live flip can re-arm without the admin route
+// having to know where satellite keeps its state.
+std::string g_databaseDir;
+} // namespace
+
+bool sdkAvailable() {
+#ifdef SATELLITE_HAS_SENTRY
+    return true;
+#else
+    return false;
+#endif
+}
+
+void init([[maybe_unused]] bool userEnabled, [[maybe_unused]] const std::string& databaseDir) {
+    // Remembered even when the policy says no, so a later opt-in can arm
+    // without being handed the path again.
+    if (!databaseDir.empty()) { g_databaseDir = databaseDir; }
+
+    if (g_active) { return; }
+
+    const std::string envOverride = envDsn();
+    if (!shouldArm(compiledDsn(), envOverride.c_str(), userEnabled)) { return; }
+
+#ifdef SATELLITE_HAS_SENTRY
+    sentry_options_t* options = sentry_options_new();
+
+    // Leave the DSN unset when only $SENTRY_DSN is present: the SDK reads the
+    // environment itself, and setting an empty string here would override it.
+    if (compiledDsn()[0] != '\0') { sentry_options_set_dsn(options, compiledDsn()); }
+
+    // Not the working directory. A tray app launched from Explorer, or the
+    // service, has no cwd worth writing run state into.
+    sentry_options_set_database_path(options, g_databaseDir.c_str());
+
+    sentry_options_set_release(options, release());
+    sentry_options_set_environment(options, environment());
+
+    // Off in shipped builds: the SDK's debug channel is noisy and satellite
+    // already has its own log.
+    sentry_options_set_debug(options, 0);
+
+    // The crash itself is the payload. Session tracking would report every
+    // start and stop of a server meant to run unattended for weeks, which is
+    // telemetry the operator did not agree to when they ticked a box labelled
+    // "crash reports".
+    sentry_options_set_auto_session_tracking(options, 0);
+
+    // No sentry_options_set_send_default_pii() call here on purpose: in
+    // sentry-native that setter exists only under SENTRY_PLATFORM_NX, and its
+    // own documentation states that not sending PII is already the default
+    // everywhere ("If false (the default), the SDK won't add PII or other
+    // sensitive data to the payload"). Calling it would not compile on any
+    // platform satellite ships to, and would not change behaviour if it did.
+
+    if (sentry_init(options) == 0) { g_active = true; }
+#endif
+}
+
+void setEnabled(bool userEnabled) {
+    if (!userEnabled) {
+        shutdown();
+        return;
+    }
+    init(true, g_databaseDir);
+}
+
+void shutdown() {
+    if (!g_active) { return; }
+    g_active = false;
+#ifdef SATELLITE_HAS_SENTRY
+    sentry_close();
+#endif
+}
+
+bool active() { return g_active; }
+
+} // namespace satellite::crash

src/adapters/crash_adapter.h

@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: LGPL-3.0-or-later
+//
+// Binds the crash-reporting policy in core/crash_reporting.h to the Sentry
+// native SDK. The third-party header lives here rather than in src/core
+// because core is std-only by gate (scripts/check_core_purity.sh) and has to
+// stay compilable on every platform with no external surface.
+//
+// Every entry point is inert when the SDK was not found at configure time
+// (SATELLITE_HAS_SENTRY undefined), so a source checkout with no Sentry
+// package still builds and behaves.
+
+#pragma once
+
+#include <string>
+
+namespace satellite::crash {
+
+// True when this build was linked against the Sentry SDK at all. False means
+// everything below is a no-op.
+bool sdkAvailable();
+
+// Arms the SDK if the policy in core/crash_reporting.h says so. `databaseDir`
+// is where Sentry keeps its run state and any pending envelope; it must be a
+// writable absolute path that survives restarts, NOT the working directory (a
+// tray app launched from Explorer, or the service, has no predictable cwd).
+//
+// Idempotent. Safe to call when the SDK is absent or the policy says no.
+void init(bool userEnabled, const std::string& databaseDir);
+
+// Applies a live flip of the operator's opt-in, reusing the database path the
+// last init() was given.
+//
+// Disarming is immediate and deliberate: withdrawing consent has to stop the
+// next crash from being sent, not the one after that. Arming is also
+// immediate, so the switch is not a lie in either direction.
+void setEnabled(bool userEnabled);
+
+// Flushes pending events and closes the SDK. Safe when init() never armed.
+void shutdown();
+
+// True only when init() actually armed the SDK. Drives what the admin UI is
+// allowed to claim: a build with no DSN must not show "reports are being sent".
+bool active();
+
+} // namespace satellite::crash

src/core/config_json.h

@@ -29,6 +29,7 @@ inline std::string serializeConfig(const Config& cfg) {
     j["controllerAudioMic"] = cfg.controllerAudioMic;
     j["controllerAudioSpeaker"] = cfg.controllerAudioSpeaker;
     j["controllerAudioKeepDefaultDevice"] = cfg.controllerAudioKeepDefaultDevice;
+    j["crashReporting"] = cfg.crashReporting;
 
     JsonOut devices = JsonOut::array();
     for (const auto& d : cfg.pairedDevices) {
@@ -85,6 +86,10 @@ inline void parseConfigInto(const std::string& text, Config& cfg) {
     cfg.controllerAudioSpeaker = jsonBool(j, "controllerAudioSpeaker", cfg.controllerAudioSpeaker);
     cfg.controllerAudioKeepDefaultDevice =
         jsonBool(j, "controllerAudioKeepDefaultDevice", cfg.controllerAudioKeepDefaultDevice);
+    // Absent in every config written before crash reporting existed, and the
+    // default it falls back to is false. That is the point: an upgrade must
+    // not start transmitting on behalf of an operator who never saw the ask.
+    cfg.crashReporting = jsonBool(j, "crashReporting", cfg.crashReporting);
 
     auto it = j.find("pairedDevices");
     if (it != j.end() && it->is_array()) {