[CrashReporting] Real send path with consent gating (DSN still placeholder)

require_user_consent + give/revoke around the Ask dialog, message event
with in-memory crash-log.txt bytes attachment, before_send scrubs
message/logentry/breadcrumbs/data of cockatrice:// secrets. Failure box
offers Save (shows path) / Discard (revokes + rejects) / Cancel (back
to dialog); reject() override so X/Escape cannot bypass revocation;
silent AlwaysSend path falls back to a local file. Empty DSN keeps the
previous mock behavior byte-identical.

Tests: 528/528 ctest green, format clean, code + UI gates PASS.
This commit is contained in:
Lukas Brübach 2026-10-14 09:25:05 +02:00 • committed by GitHub
parent 0d3750f2be
commit 038e56aab6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 331 additions and 34 deletions

View file

@ -77,6 +77,7 @@ if(WITH_CRASH_REPORTING AND WITH_CLIENT)
sentry
GIT_REPOSITORY https://github.com/getsentry/sentry-native.git
GIT_TAG ${COCKATRICE_SENTRY_TAG}
GIT_SHALLOW TRUE
)
fetchcontent_makeavailable(sentry)

View file

@ -4,15 +4,153 @@
#include <sentry.h>
#endif
#include <QByteArray>
#include <QDir>
#include <QFile>
#include <QLoggingCategory>
#include <QRegularExpression>
#include <QUrl>
#include <cstring>
#include <libcockatrice/settings/personal_settings.h>
#include <string>
Q_LOGGING_CATEGORY(CrashReportLog, "crash_report");
#if COCKATRICE_WITH_CRASH_REPORTING
namespace
{
constexpr char CockatriceUrlScheme[] = "cockatrice://";
/**
* @brief Qt-free cockatrice:// secret scrubber for the before_send hook.
*
* before_send does not run in async-signal context on our backends (breakpad invokes it on the
* crashing thread outside the signal handler, and crashpad/native crashes bypass the hook
* entirely), but it still avoids QString/QRegularExpression so it stays safe to call from the
* crash-handler thread. Truncates each URL at the first terminator, keeping scheme://host like
* redactCockatriceUrls does. (std::string allocates, so this is not strictly signal-safe.)
*/
std::string scrubCockatriceUrls(const char *input)
{
std::string out;
const char *pos = input;
const char *scheme = CockatriceUrlScheme;
const size_t schemeLen = std::strlen(scheme);
while (*pos != '\0') {
const char *found = std::strstr(pos, scheme);
if (found == nullptr) {
out.append(pos);
break;
}
out.append(pos, found);
const char *hostEnd = found + schemeLen;
while (*hostEnd != '\0' && *hostEnd != '/' && *hostEnd != '?' && *hostEnd != '#' && *hostEnd != ' ' &&
*hostEnd != '\t' && *hostEnd != '\n' && *hostEnd != '\'' && *hostEnd != '"' && *hostEnd != '<' &&
*hostEnd != '>') {
++hostEnd;
}
out.append(found, hostEnd);
pos = hostEnd;
}
return out;
}
void scrubStringValue(sentry_value_t holder, const char *key)
{
sentry_value_t current = sentry_value_get_by_key(holder, key);
if (sentry_value_get_type(current) != SENTRY_VALUE_TYPE_STRING) {
return;
}
const char *raw = sentry_value_as_string(current);
if (raw == nullptr || std::strstr(raw, CockatriceUrlScheme) == nullptr) {
return;
}
sentry_value_set_by_key(holder, key, sentry_value_new_string(scrubCockatriceUrls(raw).c_str()));
}
/**
* @brief Scrub trivially reachable string values inside a breadcrumb data object.
*/
int scrubDataEntry(const char *key, sentry_value_t value, void *userdata)
{
if (sentry_value_get_type(value) != SENTRY_VALUE_TYPE_STRING) {
return 0;
}
const char *raw = sentry_value_as_string(value);
if (raw == nullptr || std::strstr(raw, CockatriceUrlScheme) == nullptr) {
return 0;
}
auto *holder = static_cast<sentry_value_t *>(userdata);
sentry_value_set_by_key(*holder, key, sentry_value_new_string(scrubCockatriceUrls(raw).c_str()));
return 0;
}
void scrubDataObject(sentry_value_t data)
{
if (sentry_value_get_type(data) != SENTRY_VALUE_TYPE_OBJECT) {
return;
}
sentry_value_foreach_key_value(data, scrubDataEntry, &data);
}
void scrubCrumbList(sentry_value_t crumbs)
{
const size_t count = sentry_value_get_length(crumbs);
for (size_t i = 0; i < count; ++i) {
sentry_value_t crumb = sentry_value_get_by_index(crumbs, i);
if (sentry_value_get_type(crumb) != SENTRY_VALUE_TYPE_OBJECT) {
continue;
}
scrubStringValue(crumb, "message");
scrubDataObject(sentry_value_get_by_key(crumb, "data"));
}
}
void scrubBreadcrumbs(sentry_value_t event)
{
// The SDK merges scope breadcrumbs onto the event as a plain list (sentry__value_merge_breadcrumbs
// in sentry_scope.c); the { "values": [...] } envelope shape only exists on the wire. Accept both
// so the scrubbing runs on the actual merged form.
sentry_value_t breadcrumbs = sentry_value_get_by_key(event, "breadcrumbs");
if (sentry_value_get_type(breadcrumbs) == SENTRY_VALUE_TYPE_LIST) {
scrubCrumbList(breadcrumbs);
return;
}
if (sentry_value_get_type(breadcrumbs) == SENTRY_VALUE_TYPE_OBJECT) {
sentry_value_t values = sentry_value_get_by_key(breadcrumbs, "values");
if (sentry_value_get_type(values) == SENTRY_VALUE_TYPE_LIST) {
scrubCrumbList(values);
}
}
}
/**
* @brief before_send hook: scrub cockatrice:// secrets from the event.
*
* Defense in depth — payloads we build are pre-redacted. No breadcrumbs are recorded today (the Qt
* integration stays off), so this currently covers the top-level message and logentry.formatted,
* plus any breadcrumb messages/data that reach the merged event in the future. Native crash
* minidumps bypass this hook regardless of backend: breakpad captures them in-process without
* invoking before_send, and fast-fail crashes on Windows skip the local handler, so before_send is
* never invoked for those.
*/
sentry_value_t scrubEventBeforeSend(sentry_value_t event, sentry_hint_t *hint, void *userData)
{
Q_UNUSED(hint);
Q_UNUSED(userData);
scrubStringValue(event, "message");
sentry_value_t logentry = sentry_value_get_by_key(event, "logentry");
if (sentry_value_get_type(logentry) == SENTRY_VALUE_TYPE_OBJECT) {
scrubStringValue(logentry, "formatted");
}
scrubBreadcrumbs(event);
return event;
}
} // namespace
#endif
bool CrashReporter::isReportingEnabled(const QString &dsn, PersonalSettings::CrashReportingMode consentMode)
{
return !dsn.isEmpty() && consentMode != PersonalSettings::CrashReportingMode::NeverSend;
@ -46,6 +184,7 @@ void CrashReporter::init(const QString &dsn,
// Consent-gated uploads: crashes are captured into the local database but
// nothing leaves the machine until giveConsent().
sentry_options_set_require_user_consent(options, 1);
sentry_options_set_before_send(options, scrubEventBeforeSend, nullptr);
ready = (sentry_init(options) == 0);
#else
Q_UNUSED(release);
@ -80,12 +219,64 @@ void CrashReporter::giveConsent()
void CrashReporter::revokeConsent()
{
#if COCKATRICE_WITH_CRASH_REPORTING
// Revoke (not reset): an explicit refusal blocks uploads; reset would
// return to unknown, which blocks uploads too but loses the decision.
if (ready) {
sentry_user_consent_revoke();
}
#endif
}
bool CrashReporter::sendReport(const QString &redactedPreview)
{
#if COCKATRICE_WITH_CRASH_REPORTING
if (!ready) {
return false;
}
// Local scope (not the global one): the log preview rides only on this event. A global
// sentry_attach_bytes would linger and ride along on every later event in the session,
// including a real native crash later on — and with the crashpad backend byte attachments
// are written to disk for exactly that. In-memory bytes also mean no report files land in
// the Sentry DB dir, so there is no disk growth and no same-second filename collision.
sentry_scope_t *scope = sentry_scope_new();
if (scope == nullptr) {
qCWarning(CrashReportLog) << "Could not create scope for crash report";
return false;
}
const QByteArray previewBytes = redactedPreview.toUtf8();
const sentry_uuid_t attachmentId = sentry_scope_attach_bytes(
scope, previewBytes.constData(), static_cast<size_t>(previewBytes.size()), "crash-log.txt");
const bool attachOk = sentry_uuid_is_nil(&attachmentId) == 0;
if (!attachOk) {
qCWarning(CrashReportLog) << "Could not attach crash log to crash report";
}
// Release is attached automatically from the init options. Deliberately no
// sentry_set_user call: the report must carry no PII. The dialog collects
// no contact info, so the message event + attachment is the whole payload
// (sentry_value_new_feedback/sentry_capture_feedback exist but stay unused).
sentry_value_t event = sentry_value_new_message_event(SENTRY_LEVEL_ERROR, "crash-report", "Crash report");
const sentry_uuid_t eventId = sentry_scope_capture_event(scope, event, nullptr);
sentry_scope_free(scope);
if (sentry_uuid_is_nil(&eventId)) {
qCWarning(CrashReportLog) << "Failed to capture crash report event";
return false;
}
char eventIdStr[37] = {};
sentry_uuid_as_string(&eventId, eventIdStr);
// Capture only queues the envelope to the background transport: a network failure stays
// invisible here, so true means "queued", not "delivered". The Save fallback is offered only
// when the local capture itself fails.
qCInfo(CrashReportLog) << "Crash report queued, event id:" << eventIdStr;
// A failed attachment still captures the event above, but reports failure
// so callers offer the local-save fallback instead of silent success.
return attachOk;
#else
Q_UNUSED(redactedPreview);
return false;
#endif
}
QString CrashReporter::sanitizeLogLine(const QString &line)
{
static const auto colorEscapeCodePattern = QRegularExpression(QStringLiteral("\033\\[[0-9;]*m"));

View file

@ -26,12 +26,11 @@ Q_DECLARE_LOGGING_CATEGORY(CrashReportLog);
* Single responsibility: init/shutdown the sentry-native SDK behind consent
* gating, plus the shared log redaction helpers. An empty DSN (or Never
* consent) skips the SDK entirely, so the mock dialog fallback behaves exactly
* as before. init() arms require-user-consent so cached crash envelopes never
* upload before the user is asked; giveConsent()/revokeConsent() (driven by
* the reporting-mode setting and the crash dialog) gate uploads, and any stale
* persisted consent from a previous AlwaysSend run is cleared before init when
* the mode is not AlwaysSend. The real send path is a later increment. All SDK
* calls are guarded so OFF builds compile to no-ops.
* as before. With a DSN, init() arms require-user-consent and before_send
* scrubbing (stale persisted consent from a previous AlwaysSend run is cleared
* before init when the mode is not AlwaysSend); giveConsent()/revokeConsent()
* gate uploads, and sendReport() captures a redacted message event with the log
* preview attached. All SDK calls are guarded so OFF builds compile to no-ops.
*/
class CrashReporter
{
@ -49,6 +48,7 @@ public:
void shutdown();
void giveConsent();
void revokeConsent();
bool sendReport(const QString &redactedPreview);
/**
* @brief SDK-independent gating decision, unit-testable in OFF builds.

View file

@ -15,12 +15,14 @@
#include <QJsonObject>
#include <QLabel>
#include <QList>
#include <QMessageBox>
#include <QPlainTextEdit>
#include <QPushButton>
#include <QVBoxLayout>
#include <libcockatrice/settings/personal_settings.h>
DlgCrashReport::DlgCrashReport(QWidget *parent) : QDialog(parent)
DlgCrashReport::DlgCrashReport(QWidget *parent, bool _realSending, CrashReporter *_reporter)
: QDialog(parent), realSending(_realSending), reporter(_reporter)
{
infoLabel = new QLabel(this);
infoLabel->setWordWrap(true);
@ -70,9 +72,26 @@ void DlgCrashReport::changeEvent(QEvent *event)
void DlgCrashReport::retranslateUi()
{
setWindowTitle(tr("Send crash report"));
infoLabel->setText(
tr("Cockatrice closed unexpectedly. The preview below shows what a crash report would contain (mock)."));
accountLabel->setText(tr("No account needed to send a crash report."));
if (realSending) {
// The preview below prefers the crashed session's persisted log tail
// (Logger::getPreviousSessionTail), falling back to this restart's in-memory buffer only
// when no previous session was saved. The native crash data (minidump + version/OS
// metadata cached in the Sentry DB dir by breakpad) is flushed separately on
// giveConsent(); the preview is attached as crash-log.txt.
infoLabel->setText(
tr("Cockatrice closed unexpectedly. Sending uploads the stored crash data from the previous "
"run (crash dump plus app version and operating system info) together with the redacted "
"log excerpt shown below (last lines from the crashed session, attached as crash-log.txt). "
"If no log from the crashed session was saved, this restart's early logs are shown instead."));
} else {
infoLabel->setText(
tr("Cockatrice closed unexpectedly (mock preview, nothing will be uploaded). A real report "
"would contain the stored crash data from the previous run plus the redacted log excerpt "
"shown below (last lines from the crashed session). If no log from the crashed session was "
"saved, this restart's early logs are shown instead. A mock file is saved locally instead."));
}
accountLabel->setText(tr("No account needed, and no contact info is collected. Login links "
"(cockatrice://...) are scrubbed before sending."));
rememberBox->setText(tr("Remember my choice"));
sendButton->setText(tr("Send"));
dontSendButton->setText(tr("Don't send"));
@ -134,7 +153,48 @@ void DlgCrashReport::actSend()
if (rememberBox->isChecked()) {
SettingsCache::instance().personal().setCrashReportingMode(PersonalSettings::CrashReportingMode::AlwaysSend);
}
const QString savedPath = writeMockReport(previewArea->toPlainText());
const QString preview = previewArea->toPlainText();
if (realSending && reporter != nullptr) {
reporter->giveConsent();
if (reporter->sendReport(preview)) {
accept();
return;
}
qCWarning(CrashReportLog) << "Failed to send crash report";
// Outcome mapping for the failure box: Save = store locally and close
// as accepted (the follow-up info box shows the path, so the user
// knows it was NOT sent); Discard = revoke consent and close as
// rejected (dropped); Cancel/X = back to the dialog, neither sent nor
// dropped. The Remember checkbox governs only the original
// Send/Don't-send decision, so the Discard path below revokes and
// closes via QDialog::reject() without touching the setting (calling
// our reject() override here would wrongly persist NeverSend over the
// AlwaysSend stored above when Remember was checked).
QMessageBox failureBox(QMessageBox::Warning, tr("Send crash report"),
tr("Sending the crash report failed. Save the report locally instead?"),
QMessageBox::Save | QMessageBox::Discard | QMessageBox::Cancel, this);
failureBox.setDefaultButton(QMessageBox::Save);
failureBox.setEscapeButton(QMessageBox::Cancel);
const QMessageBox::StandardButton choice = static_cast<QMessageBox::StandardButton>(failureBox.exec());
if (choice == QMessageBox::Save) {
const QString savedPath = writeMockReport(preview);
if (savedPath.isEmpty()) {
QMessageBox::warning(this, tr("Send crash report"), tr("Could not save the crash report locally."));
} else {
QMessageBox::information(this, tr("Send crash report"),
tr("Crash report saved locally to:\n%1").arg(savedPath));
}
accept();
return;
}
if (choice == QMessageBox::Discard) {
reporter->revokeConsent();
QDialog::reject();
return;
}
return;
}
const QString savedPath = writeMockReport(preview);
if (savedPath.isEmpty()) {
qCWarning(CrashReportLog) << "Could not save the mock crash report locally";
} else {
@ -144,9 +204,17 @@ void DlgCrashReport::actSend()
}
void DlgCrashReport::actDontSend()
{
reject();
}
void DlgCrashReport::reject()
{
if (rememberBox->isChecked()) {
SettingsCache::instance().personal().setCrashReportingMode(PersonalSettings::CrashReportingMode::NeverSend);
}
reject();
if (realSending && reporter != nullptr) {
reporter->revokeConsent();
}
QDialog::reject();
}

View file

@ -15,21 +15,27 @@ class QLabel;
class QPlainTextEdit;
class QPushButton;
class CrashReporter;
/**
* @brief MOCK crash-report dialog (UI + settings only, no network).
* @brief Crash-report dialog: mock fallback or real Sentry send.
*
* The Send path writes a redacted JSON file to the temp dir instead of
* uploading anything. Real Sentry upload comes in a later phase.
* Mock mode (default): the Send path writes a redacted JSON file to the temp
* dir instead of uploading anything. Real mode (realSending, with a live
* CrashReporter): Send gives consent and captures a redacted message event
* with the log preview attached; failure offers a local save instead.
*/
class DlgCrashReport : public QDialog
{
Q_OBJECT
public:
explicit DlgCrashReport(QWidget *parent = nullptr);
explicit DlgCrashReport(QWidget *parent = nullptr, bool realSending = false, CrashReporter *_reporter = nullptr);
static QString buildRedactedPreview();
static QString writeMockReport(const QString &previewText);
void reject() override;
protected:
void changeEvent(QEvent *event) override;
@ -40,6 +46,8 @@ private:
QCheckBox *rememberBox;
QPushButton *sendButton;
QPushButton *dontSendButton;
bool realSending = false;
CrashReporter *reporter = nullptr;
void retranslateUi();

View file

@ -264,7 +264,7 @@ int main(int argc, char *argv[])
qApp->setAttribute(Qt::AA_DontShowIconsInMenus, true);
#endif
// Crash reporting (increment 1): init the SDK early so a crash anywhere below is captured into the
// Crash reporting: init the SDK early so a crash anywhere below is captured into the
// local database. Uploads stay consent-gated (require_user_consent, armed in CrashReporter::init):
// AlwaysSend gives consent here so cached envelopes flush at startup; any other mode starts
// unconsented and the dialog below drives give/revoke. An empty DSN skips the SDK entirely and
@ -274,13 +274,13 @@ int main(int argc, char *argv[])
{
QString crashDsn = QString::fromUtf8(qgetenv("COCKATRICE_SENTRY_DSN"));
if (crashDsn.isEmpty()) {
crashDsn = QString::fromLatin1(COCKATRICE_SENTRY_DSN);
crashDsn = QString::fromUtf8(COCKATRICE_SENTRY_DSN);
}
crashReporter.init(crashDsn, QString::fromLatin1(VERSION_STRING), crashDatabasePath,
crashReporter.init(crashDsn, QString::fromUtf8(VERSION_STRING), crashDatabasePath,
SettingsCache::instance().personal().getCrashReportingMode());
if (crashReporter.isReady() && SettingsCache::instance().personal().getCrashReportingMode() ==
PersonalSettings::CrashReportingMode::AlwaysSend) {
// Silent opt-in: cached crash envelopes flush automatically once consent is given.
// Silent opt-in: cached native crash envelopes flush automatically once consent is given.
crashReporter.giveConsent();
}
// Mid-session mode changes must reach the SDK: without this, switching from AlwaysSend to
@ -323,7 +323,8 @@ int main(int argc, char *argv[])
{{{"c", "connect"}, QCoreApplication::translate("main", "Connect on startup"), "user:pass@host:port"},
{{"d", "debug-output"}, QCoreApplication::translate("main", "Debug to file")},
{"simulate-crash",
QCoreApplication::translate("main", "Show the mock crash-report dialog (nothing is sent)")}});
QCoreApplication::translate("main",
"Show the crash-report dialog (saves a mock file when no DSN is configured)")}});
parser.process(app);
@ -460,10 +461,11 @@ int main(int argc, char *argv[])
ui.show();
qCInfo(MainLog) << "ui.show() finished";
// MOCK crash-report flow (UI + settings only): no sentry-native upload, no network.
// Triggered by --simulate-crash or by leftover Windows crash dumps. Handled dumps are deleted
// after Send / Don't send (or a silent NeverSend skip) so one crash never prompts twice; on
// non-Windows nothing writes cockatrice.crash.*.dmp yet, so --simulate-crash is the trigger there.
// Crash-report flow: with a configured DSN the SDK owns capture and upload
// (consent-gated); with an empty DSN the dialog falls back to a local mock
// file and behaves exactly as before. Triggered by --simulate-crash or by
// leftover Windows crash dumps, which are consumed after handling so one
// crash never prompts twice.
{
QString crashDumpDir;
#ifdef Q_OS_WIN
@ -487,18 +489,28 @@ int main(int argc, char *argv[])
if (parser.isSet("simulate-crash") || !crashDumps.isEmpty()) {
const PersonalSettings::CrashReportingMode crashMode =
SettingsCache::instance().personal().getCrashReportingMode();
const bool crashReady = crashReporter.isReady();
if (crashMode == PersonalSettings::CrashReportingMode::AlwaysSend) {
crashReporter.giveConsent();
DlgCrashReport::writeMockReport(DlgCrashReport::buildRedactedPreview());
} else if (crashMode == PersonalSettings::CrashReportingMode::Ask) {
DlgCrashReport crashDialog(&ui);
if (crashDialog.exec() == QDialog::Accepted) {
crashReporter.giveConsent();
if (crashReady) {
// Silent path: consent was given at startup, so native crash
// envelopes flush automatically; this sends the log preview
// event on top.
const QString crashPreview = DlgCrashReport::buildRedactedPreview();
if (!crashReporter.sendReport(crashPreview)) {
const QString savedPath = DlgCrashReport::writeMockReport(crashPreview);
qCWarning(MainLog) << "Failed to send crash report, saved locally to" << savedPath;
}
} else {
crashReporter.revokeConsent();
DlgCrashReport::writeMockReport(DlgCrashReport::buildRedactedPreview());
}
} else if (crashMode == PersonalSettings::CrashReportingMode::Ask) {
// Real mode: Send gives consent + captures the event, Don't
// send revokes consent; the dialog owns both via the reporter.
DlgCrashReport crashDialog(&ui, crashReady, crashReady ? &crashReporter : nullptr);
crashDialog.exec();
}
// NeverSend: skip silently (no dialog, nothing written or sent).
// Handled dumps are consumed so one crash never prompts twice.
for (const QString &dump : crashDumps) {
QFile::remove(crashDumpDir + QStringLiteral("/") + dump);
}

View file

@ -1,5 +1,6 @@
#include "client/crash/crash_reporter.h"
#include <QDir>
#include <QString>
#include <gtest/gtest.h>
#include <libcockatrice/settings/personal_settings.h>
@ -39,7 +40,7 @@ TEST(CrashReporterTest, RedactCockatriceUrlsLeavesOtherTextAlone)
TEST(CrashReporterTest, InitSkippedWithoutDsn)
{
CrashReporter reporter;
reporter.init(QString(), QStringLiteral("3.1.2"), QStringLiteral("/tmp/cockatrice-crash-test"),
reporter.init(QString(), QStringLiteral("3.1.2"), QDir::tempPath() + QStringLiteral("/cockatrice-crash-test"),
PersonalSettings::CrashReportingMode::Ask);
ASSERT_FALSE(reporter.isReady());
reporter.shutdown();
@ -50,7 +51,8 @@ TEST(CrashReporterTest, InitSkippedWhenNeverSend)
{
CrashReporter reporter;
reporter.init(QStringLiteral("https://key@sentry.io/1"), QStringLiteral("3.1.2"),
QStringLiteral("/tmp/cockatrice-crash-test"), PersonalSettings::CrashReportingMode::NeverSend);
QDir::tempPath() + QStringLiteral("/cockatrice-crash-test"),
PersonalSettings::CrashReportingMode::NeverSend);
ASSERT_FALSE(reporter.isReady());
reporter.shutdown();
}
@ -78,6 +80,21 @@ TEST(CrashReporterTest, ShutdownWithoutInitIsSafe)
ASSERT_FALSE(reporter.isReady());
}
TEST(CrashReporterTest, ConsentAndSendWithoutDsnAreSafeNoOps)
{
CrashReporter reporter;
reporter.init(QString(), QStringLiteral("3.1.2"), QDir::tempPath() + QStringLiteral("/cockatrice-crash-test"),
PersonalSettings::CrashReportingMode::Ask);
ASSERT_FALSE(reporter.isReady());
reporter.giveConsent();
reporter.revokeConsent();
ASSERT_FALSE(reporter.isReady());
ASSERT_FALSE(reporter.sendReport(QStringLiteral("preview")));
ASSERT_FALSE(reporter.sendReport(QString()));
reporter.shutdown();
ASSERT_FALSE(reporter.isReady());
}
} // namespace
int main(int argc, char **argv)