Bug 2059588 - Invoke crash reporter from Felt to add bearer authentication to crash reports

[jonathanmendez]

Description

Bugzilla: Bug-2059588

TODO: This is WIP for an alternative to #1256

---

Dependencies / Related Issues

• Alternative to: Bug 2059588 - Add console bearer authentication to crash reports/pings #1256

---

Testing

• Added tests
• Manual testing performed

[Copilot]

Pull request overview

Adds Felt bearer-token authentication to crash reports and crash pings, including token propagation and authenticated upload paths.

Changes:

• Synchronizes tokens with crash reporter processes.
• Adds authenticated upload headers and retry handling.
• Adds networking tests and HTTP dependency updates.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Summary
xpcom/system/nsICrashReporter.idl	Adds the authentication-token API.
toolkit/xre/nsAppRunner.cpp	Bridges token configuration to crash reporting.
toolkit/crashreporter/nsExceptionHandler.h	Declares token handling.
toolkit/crashreporter/nsExceptionHandler.cpp	Passes tokens to crash reporter children.
toolkit/crashreporter/networking/tests/xpcshell/test_BackgroundTask_crashreporterNetworkBackend.js	Tests header forwarding.
toolkit/crashreporter/networking/BackgroundTask_crashreporterNetworkBackend.sys.mjs	Forwards serialized request headers.
toolkit/crashreporter/CrashSubmit.sys.mjs	Adds authenticated submissions and retries.
toolkit/crashreporter/client/app/src/test.rs	Adds enterprise transport tests.
toolkit/crashreporter/client/app/src/net/report.rs	Authenticates crash report uploads.
toolkit/crashreporter/client/app/src/net/mod.rs	Registers networking modules.
toolkit/crashreporter/client/app/src/net/http.rs	Adds HTTP header handling.
toolkit/crashreporter/client/app/src/net/auth.rs	Reads enterprise authentication tokens.
toolkit/crashreporter/client/app/src/logic.rs	Supplies report authentication headers.
toolkit/crashreporter/client/app/src/glean.rs	Supplies authentication headers for pings.
toolkit/crashreporter/client/app/Cargo.toml	Adds the HTTP dependency.
toolkit/components/enterprise/modules/ConsoleClient.sys.mjs	Synchronizes Felt tokens.
Cargo.lock	Locks dependency updates.

Suppressed comments (3)

toolkit/crashreporter/client/app/src/glean.rs:206

• This refactor changes RequestBuilder::build failures from recoverable to unrecoverable. Build can fail for transient request-file or backend-spawn errors, and the previous code retried these failures; marking them unrecoverable causes Glean to permanently drop the ping instead.

                        UploadResult::unrecoverable_failure()

toolkit/crashreporter/client/app/src/net/http.rs:167

• These serialized headers are written to TempRequestFile by the background-task path. That file is created under the system temp directory with a predictable name and default permissions, so adding the bearer value here makes it readable by other local processes while the task runs. Use a securely created, owner-only temporary file (with exclusive/random naming) or avoid writing the credential to disk.

/// Serialize a [`HeaderMap`] as an array of `[name, value]` pairs, matching the
/// JSON format expected by `BackgroundTask_crashreporterNetworkBackend.sys.mjs`.
fn serialize_headers<S>(headers: &HeaderMap, serializer: S) -> Result<S::Ok, S::Error>

toolkit/crashreporter/nsExceptionHandler.cpp:236

• gAuthTokenEnvEntry is read by LaunchProgram on the crash path, while _syncCrashReporterAuthToken() can mutate it on the main thread whenever Felt refreshes a token. An nsCString read concurrent with Assign/Append or Truncate is a data race, and a reallocation can leave the crash path with an invalid or partial environment entry. Use an immutable/atomic snapshot or another crash-handler-safe handoff for token updates.

    MOZ_GUARDED_BY(gCrashHelperClientMutex) = nullptr;
static google_breakpad::ExceptionHandler* gExceptionHandler = nullptr;
static mozilla::Atomic<bool> gEncounteredChildException(false);
constinit static nsCString gServerURL;

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.

Suppressed comments (4)

toolkit/components/felt/FeltCrashReporter.sys.mjs:67

• This check only honors MOZ_CRASHREPORTER_NO_REPORT, but ShouldReport() also suppresses submission when MOZ_CRASHREPORTER_FULLDUMP is set (and the crash reporter documentation says full dumps must not be submitted). On Windows, the Felt path can therefore upload a full dump despite that setting; include the full-dump environment variable in this gate.

    if (Services.env.get("MOZ_CRASHREPORTER_NO_REPORT")) {
      // The browser inherits our environment, so this is the same value that
      // would have stopped it from launching the client itself.
      lazy.log.debug("report: crash reporting disabled in the environment");
      return false;
    }

toolkit/components/felt/FeltCrashReporter.sys.mjs:174

• The high-water mark uses only the dump's modification time as its identity. On filesystems with coarse timestamps, two crashes can have the same lastModified; after the first is reported, the lastModified <= newestTime check rejects the second permanently. Track reported/in-flight paths or another unique dump identity instead of relying on mtime alone.

        lastModified <= newestTime

toolkit/crashreporter/client/app/src/glean.rs:206

• RequestBuilder::build() can fail while creating the background-task request file or invoking the available curl/libcurl backends, so this is not limited to a malformed URL. Returning unrecoverable_failure() here discards transient crash pings instead of allowing Glean to retry, unlike the previous behavior; keep this result recoverable (or distinguish malformed configuration explicitly).

                    Err(e) => {
                        log::error!("failed to build request for glean ping: {e}");
                        UploadResult::unrecoverable_failure()

toolkit/crashreporter/client/app/src/net/http.rs:324

• Passing the Authorization value through cmd.args(["--header", ...]) exposes the bearer token in the curl process command line while the upload is running (for example via /proc/<pid>/cmdline). That defeats the sensitive-header redaction above and allows other local processes to read the console credential; use libcurl or a protected config/IPC path rather than putting the token in argv.

                for (name, value) in headers.iter() {
                    cmd.args(["--header", &curl_header_line(name, value)]);

[afranchuk]

This looks reasonable, but it is yet another different crash path to maintain.

[afranchuk]

Suggested change

	* leaves the submission to us, so that the upload can be authenticated with a

s/client/submission. At the time of writing, GH isn't rendering this suggestion correctly, so hopefully this change is clear.

[afranchuk]

If the crash reporter fails (non-zero exit code), there's no handling of this case. It should probably throw, since calling code expects an exception on failure.

[afranchuk]

I think proc.wait() is better than using exitPromise (though they have the same result as currently implemented).

[afranchuk]

This should be scope: client (the default if scope is omitted). It shouldn't actually be sent in reports.