jeirmeister/Cockatrice
1
0
Fork 0
mirror of https://github.com/Cockatrice/Cockatrice.git synced 2026-11-12 21:10:01 -08:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

[CrashReporting] Sentry send path with consent gating (#7522)

Browse source 
* [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.

* [CrashReporting] Attach crash log and user note to the pending crash event

The redacted log used to ride a separate message event from the native
crash (minidump + stack), forcing manual correlation in Sentry. Fold it
into the pending crash envelope(s) instead by appending well-formed
attachment items before consent releases them: sentry-native replays
cached envelopes byte-for-byte (raw read, raw serialize in 0.17.3), so
the appended crash-log.txt and user-note.txt items upload with the very
event that already carries the dump and stack.

Covers both storage locations: Ask mode's consent-held files in
<database>/cache and AlwaysSend's transport-dumped files in
<database>/<run>.run, which must be attached before sentry_init replays
them. When no pending crash event exists (unclean exit without a native
crash) the standalone message event remains as fallback, now also
carrying the optional note.

The dialog gains an optional free-text field ('Here's what I was
doing...') that travels as user-note.txt; the mock JSON records it too.

* [CrashReporting] Send PROJECT_VERSION as the Sentry release

main() reported VERSION_STRING (PROJECT_VERSION_FRIENDLY) as the Sentry
release — that carries a build date, so every build of the same version
landed in its own release, and it never matched the installer, which
reports CPACK/PROJECT_VERSION. Bake COCKATRICE_SENTRY_RELEASE=
PROJECT_VERSION instead: date-free, identical between client and
installer, and derived from the git tag on tag builds, so events,
symbol uploads and the installer log all group under one release.

* [CrashReporting] Friendlier crash dialog copy

Reword the report dialog along a single friendly arc: headline ('Oops,
we crashed — and now we need your help to make Cockatrice better!'),
why helping is easy, exactly what gets collected (crash dump with stack
trace, app version, OS info, redacted log excerpt), the value of each
report ('better and less crashy for everyone'), and the consent choice
('Send and remember makes this automatic without interruption in the
future'). The checkbox itself stays direction-neutral ('Remember my
choice') because it also persists a 'Don't send' decision.

* [CrashReporting] Log where crash reports go and warn when they cannot

The client resolved COCKATRICE_SENTRY_DSN (environment over the baked-in
value) without ever saying which one won, and sentry-native refuses to build
a request for a DSN it considers invalid without reporting anything, so a
tester pointing the client at a scratch project had nothing to go on when a
crash never appeared.

Say which DSN source won, and log the destination (host and project id, never
the key) once the SDK is armed. Warn instead of staying quiet when no DSN is
configured at all, when a DSN is malformed - disabling reporting rather than
silently falling back to the project baked into the build, which is the rule
the installer already follows in cmake/CrashReporting.cmake - and when the
build was compiled without Sentry support. A deliberate Never-send stays at
info level: that is a choice, not a misconfiguration.

Also warn when a pending crash envelope was captured for a different
destination than the session that will upload it. sentry-native posts every
envelope with the DSN held in the transport state of the session doing the
sending, and envelopes restored from disk are raw, so the "dsn" header of a
cached envelope is never read: a crash captured for one project can be
uploaded to another with nothing in the log to show it.

Tests cover the DSN shape check (same unanchored pattern the build matches, so
a DSN this build accepted is never rejected here), destination description
without the key, and the mismatch detection, which stays quiet for a different
key on the same project, unreadable files, and headers without a dsn.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
BruebachL 2026-10-19 14:57:03 +02:00 • committed by GitHub
parent 6385f3fab3
commit ad0fd2d4b0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 965 additions and 47 deletions
Download patch file Download diff file Expand all files Collapse all files

97
cockatrice/src/client/crash/crash_reporter.h
View file

@ -6,8 +6,10 @@
#ifndef CRASH_REPORTER_H
#define CRASH_REPORTER_H
#include <QByteArray>
#include <QLoggingCategory>
#include <QString>
#include <QStringList>
#include <libcockatrice/settings/personal_settings.h>
#ifndef COCKATRICE_WITH_CRASH_REPORTING
@ -18,6 +20,10 @@
#define COCKATRICE_SENTRY_DSN ""
#endif
#ifndef COCKATRICE_SENTRY_RELEASE
#define COCKATRICE_SENTRY_RELEASE ""
#endif
Q_DECLARE_LOGGING_CATEGORY(CrashReportLog);
/**
@ -26,12 +32,15 @@ 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. attachToPendingCrashEnvelopes() folds the redacted log and the
* optional user note into the pending native crash event(s) before consent
* releases them, so dump, stack, log and note arrive as ONE Sentry event;
* sendReport() is the fallback for unclean exits that produced no native crash
* event, capturing a redacted message event with the log preview attached. All
* SDK calls are guarded so OFF builds compile to no-ops.
*/
class CrashReporter
{
@ -49,12 +58,88 @@ public:
void shutdown();
void giveConsent();
void revokeConsent();
bool sendReport(const QString &redactedPreview, const QString &userNote = QString());
/**
* @brief Append the redacted log (and optional user note) to every pending
* native crash envelope in the local database, so the crashed event uploads
* with dump, stack, log and note together instead of the log arriving as a
* separate event that has to be correlated by hand. Both storage locations
* are covered: consent-held envelopes in <database>/cache (Ask mode) and
* in-flight envelopes dumped into <database>/<run>.run (AlwaysSend mode).
*
* For the AlwaysSend variant run this BEFORE init() (static overload): the
* startup replay of *.run envelopes happens inside sentry_init. SDK-independent
* plain-file surgery on the Sentry envelope format, so it stays unit-testable
* in OFF builds.
*
* @return the number of envelopes that received the attachments.
*/
int attachToPendingCrashEnvelopes(const QString &redactedLogText, const QString &userNoteText);
static int attachToPendingCrashEnvelopes(const QString &databasePath,
const QString &redactedLogText,
const QString &userNoteText);
/**
* @brief Append one attachment item to a cached Sentry envelope file.
*
* An envelope is a newline-separated stream of item-header-line + payload
* pairs. The retry transport replays cached envelopes byte-for-byte
* (sentry-native 0.17.3 reads them raw and serializes raw envelopes back
* verbatim), so appending well-formed items associates them with the event
* already in the file; the first line (event id header) is never touched.
*
* @param filename ASCII-only item filename, embedded verbatim in the header.
*/
static bool
appendAttachmentToEnvelope(const QString &envelopePath, const QString &filename, const QByteArray &payload);
/**
* @brief Pending crash envelopes in <database>: the consent-held files in
* <database>/cache and the in-flight files in <database>/<run>.run.
*
* Matches the cache filename conventions sentry-native accepts (see
* sentry__parse_cache_filename): bare "<uuid>.envelope" and retry
* "<ts>-<count>-<uuid>.envelope"; run dirs accept plain "<uuid>.envelope"
* minus the SDK's own "__sentry-crash.envelope" marker.
*/
static QStringList pendingEnvelopePaths(const QString &databasePath);
/**
* @brief SDK-independent gating decision, unit-testable in OFF builds.
*/
static bool isReportingEnabled(const QString &dsn, PersonalSettings::CrashReportingMode consentMode);
/**
* @brief Whether a DSN has the shape the build and the SDK require:
* <scheme>://<key>@<host>/<project-id>.
*
* Deliberately the same unanchored pattern cmake/CrashReporting.cmake matches (CMake's MATCHES
* searches rather than anchoring), so a DSN this build accepted is never rejected here. The SDK
* refuses to even build a request for a DSN it considers invalid, so an unchecked typo disables
* uploading with no error anywhere.
*/
static bool isDsnWellFormed(const QString &dsn);
/**
* @brief Human-readable destination of a DSN, e.g. "o0.ingest.sentry.io (project 1234567)".
*
* Safe to log: the public key is never part of the result. Empty for a malformed DSN.
*/
static QString describeDsn(const QString &dsn);
/**
* @brief Describe the captured-for DSN of a pending envelope when it points at a different
* destination than the DSN this session would upload it with; empty when they agree.
*
* sentry-native posts every envelope with the DSN held in the transport state of the session
* doing the sending (envelopes restored from disk are raw and their own "dsn" header is never
* read), so a cached crash envelope captured for one project can silently be uploaded to
* another - the exact way a tester's report lands in the wrong place. Compares destinations
* (host and project id), not raw strings, so a different key for the same project stays quiet.
*/
static QString describeOriginMismatch(const QString &envelopePath, const QString &currentDsn);
static QString sanitizeLogLine(const QString &line);
static QString redactCockatriceUrls(const QString &line);