Skip to content

Sentry Patches - #1

Open
jan-auer wants to merge 729 commits into
masterfrom
getsentry
Open

Sentry Patches#1
jan-auer wants to merge 729 commits into
masterfrom
getsentry

Conversation

@jan-auer

Copy link
Copy Markdown
Member

This PR contains patches for Sentry. Do not merge.

Swatinem pushed a commit that referenced this pull request Sep 10, 2020
__builtin_trap uses ud2 on x86_64, producing a SIGILL. On arm64, it uses
brk #1, producing a SIGTRAP. Test expectations must be adjusted
accordingly.

Bug: crashpad:345
Test: crashpad_snapshot_test MachOImageAnnotationsReader.CrashModuleInitialization, crashpad_util_test ExcServerVariants.*,ExceptionPorts.*
Change-Id: I22e75b7b48b8887031b1d95f1cea8a09733daf49
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2386464
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
supervacuus pushed a commit to supervacuus/crashpad that referenced this pull request Dec 12, 2022
…Body

This bug was found when trying to upgrading the MSAN bots from Ubuntu
18.04 (where this codepath was not hit) to 20.04.  The following MSAN
error is produced when running HTTPTransport/HTTPTransport.*

==3496553==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5616c540ad7d in __is_long buildtools/third_party/libc++/trunk/include/string:1674:33
    getsentry#1 0x5616c540ad7d in size buildtools/third_party/libc++/trunk/include/string:1069:17
    getsentry#2 0x5616c540ad7d in crashpad::(anonymous namespace)::HTTPTransportLibcurl::WriteResponseBody(char*, unsigned long, unsigned long, void*) third_party/crashpad/crashpad/util/net/http_transport_libcurl.cc:528:50
    ...
    SUMMARY: MemorySanitizer: use-of-uninitialized-value buildtools/third_party/libc++/trunk/include/string:1674:33 in __is_long
  ORIGIN: invalid (0). Might be a bug in MemorySanitizer origin tracking.

The memory is initialized in http_transport_test.cc:293, but MSAN gets
confused.  Given the message output by MSAN (ORIGIN: invalid (0).
Might be a bug in MemorySanitizer origin tracking), this appears
to be a bug in MSAN, not crashpad, so this CL suppresses the error.

Bug: chromium: 1260217
Change-Id: I2d6a46e3489816270cc1fee776793ffafe0147e4
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4015160
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
bhamiltoncx and others added 28 commits December 18, 2023 18:34
Change-Id: Ifc373d313db71872cc0fd7706da2bdc07cf4ba1b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5126940
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Ben Hamilton <benhamilton@google.com>
Avoid unconditionally running mig in incremental builds,
and causing un-necessary re-build of downstream targets.

Bug: b/42147841
Bug: b/42084680
Change-Id: I961189870aec8f0b1a1ced22105730218664e109
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5171755
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: David Fang <fangism@google.com>
…ae8eb0 (1 commit)

https://chromium.googlesource.com/chromium/mini_chromium/+log/ac3e73239534..cc2ae8eb01d0

$ git log ac3e73239..cc2ae8eb0 --date=short --no-merges --format='%ad %ae %s'
2024-01-10 pbos Add Flush() method to LogMessage

Created with:
  roll-dep crashpad/third_party/mini_chromium/mini_chromium

Bug: chromium:1409729
Change-Id: I59b4c9fc9701f3a504b89f396de9da49b333712f
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5185844
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
…4ecb51 (1 commit)

https://chromium.googlesource.com/chromium/mini_chromium/+log/cc2ae8eb01d0..1e64ecb51edf

$ git log cc2ae8eb0..1e64ecb51 --date=short --no-merges --format='%ad %ae %s'
2024-01-10 pbos Make ~LogMessage virtual

Created with:
  roll-dep crashpad/third_party/mini_chromium/mini_chromium

Bug: chromium:1409729
Change-Id: Ib0011b85c35c781ea35e0d399cccb81b54916ca4
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5186000
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
…ccaa8e (1 commit)

https://chromium.googlesource.com/chromium/mini_chromium/+log/1e64ecb51edf..c7fccaa8ec14

$ git log 1e64ecb51..c7fccaa8e --date=short --no-merges --format='%ad %ae %s'
2024-01-10 pbos Implement base::ImmediateCrash()

Created with:
  roll-dep crashpad/third_party/mini_chromium/mini_chromium

Bug: chromium:1409729
Change-Id: Iaa4f4d81027cb0ddafe85bab8d186949eb7f852e
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5186004
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
…a01130 (1 commit)

https://chromium.googlesource.com/chromium/mini_chromium/+log/c7fccaa8ec14..203a01130fac

$ git log c7fccaa8e..203a01130 --date=short --no-merges --format='%ad %ae %s'
2024-01-10 pbos Fix base::ImmediateCrash() IWYU

Created with:
  roll-dep crashpad/third_party/mini_chromium/mini_chromium

Bug: chromium:1409729
Change-Id: Ic9fee112a48b66c92b8446270280d5f2a3d0ef59
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5187485
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
This will be used by base/logging.h in chromium to make sure that
LOG(FATAL) variants never return and are properly understood as
[[noreturn]] by the compiler.

Once that's landed in chromium it'll be up/downstreamed into
mini_chromium as well.

Bug: chromium:1409729
Change-Id: I75340643fe075475f997bbc45250fa10df63c9fa
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5185996
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
This CL introduces a new crash key 'crashpad_uptime_ns' that records the
number of nanoseconds between when Crashpad was initialized and when a
snapshot is generated.

Crashpad minidumps record the MDRawMiscInfo process_create_time using a
sysctl(KERN_PROC).kp_proc.p_starttime. This time is used to display the
'uptime' of a process.  However, iOS 15 and later has a feature that
'prewarms' the app to reduce the amount of time the user waits before
the app is usable. This mean crashes that may happen immediately on
startup would appear to happen minutes or hours after process creation
time.

While initial implementations of prewarming would include some parts of
main, since iOS16 prewarming is complete before main, and therefore
before Crashpad is typically initialized.

Bug: crashpad:472
Change-Id: Iff960e37ae40121bd5927d319a2767d1cafce846
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5171091
Reviewed-by: Ben Hamilton <benhamilton@google.com>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
This will trigger dead-code warnings in chromium once LOG(FATAL) is
understood as [[noreturn]], which needs to be fixed in crashpad first.

Bug: 1409729
Change-Id: I75cb4d93e648ca9804f1299345e52bb3e2834cd9
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5193351
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Change-Id: I0a30b816e2550e7df6d7777c6d27e6104fc2f9fa
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5206711
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
It's expected that iOS intermediate dumps can be written with
missing information, but it's better to try and report as much as
possible rather than drop the incomplete minidump.

Bug: b/284959148
Change-Id: I04110b576a4ee552814234d559c9ba85db0382f0
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4582167
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
This assumption is non-portable and prevents Chromium from using
bounded iterators in libc++.

Bug: chromium: 1519908
Change-Id: Iafe6639ef3bc896d6fa4fb3ceb7ac0b546363017
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5237292
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
This changes fxbug.dev/ URLs from Monorail bug numbers to the new
Fuchsia Issue Tracker numbers.

The migration to the new issue tracker was announced here:
https://groups.google.com/a/fuchsia.dev/g/announce/c/GOYfJozEqmk/m/qsGsaJ7UAAAJ

Bug: 298074672
Change-Id: I5f4b7a26a3f41bf539fa79d15e1a108ea35a5b29
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5246697
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
The change in macOS 14's dyld to insert new modules in the front of
`dyld_all_image_infos` means that if the any images are loaded
during an exception while iterating the modules list, the primary
executable will be missed. Instead, read the modules in reverse
order.


Change-Id: I49f6468173f18ef4bd0f326c84e4b48cfc696cd3
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5249275
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
This change adds a method to update a CrashpadInfo stream.

As part of this change, AddUserDataMinidumpStream() now returns a handle
to the added stream. This handle can be passed to
UpdateUserDataMinidumpStream() if a new version of the stream needs to
be attached to the crash report.

This method is needed for e.g., allowing Chrome's System Profile to
update, as it contains some data that takes a while to collect.

Bug: crashpad:474
Change-Id: I19e935a6904d8843215582e5606b189479ee338b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5260024
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
If building for chromium, honor the ios_is_app_extension gn variable
that is set per toolchain. When it is defined, the code is built for
an application extension (i.e. -fapplication-extension is passed to
the compiler).

Use CRASHPAD_IS_IOS_APP_EXTENSION build guard to not compile code
that use unavailable extension when ios_is_app_extension is set. If
the variable is not set, then check at runtime whether the API can
be used or not (if the crashpad client uses the same toolchain for
the main application and its application extensions).

This is required to pass -fapplication-extension to the compiler when
building application extensions (which allow catching API that is not
available to application extensions).

Bug: 40120082
Change-Id: I28d545fcfd0f8662430c40ff202b79b0c2b2ff8b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5286216
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
CrashpadInfo::AddUserDataMinidumpStream() and
UpdateUserDataMinidumpStream() allocate memory for the newly added
streams. This change makes the CrashpadInfo test free that allocated
memory to prevent memory leaks from these tests.

This is intended to fix the ASAN failures seen on crrev.com/c/5285881:
https://ci.chromium.org/ui/p/chromium/builders/try/linux_chromium_asan_rel_ng/1839072/overview

Bug: crashpad:474
Change-Id: I6e030291594d22e316942a58805a177ce448053b
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5292137
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Bug: b/325495632
Change-Id: I19df5b44b76efcdb050344e79bcc2dfd18d8e289
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5299466
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Bug: 325296797
Change-Id: I39f76519c46804ad663172abf91ef582bde135e7
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5310754
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
This adds argv[0] for PLOG(FATAL) calls following a failed posix_spawn
or execve call to make logs more useful.

Bug: chromium:324982367
Change-Id: I179928ec9f791ce5b365b3444aa3bb667f4ec4b3
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5315332
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Chrome on ChromeOS is starting to run into problems where there are more
than 200 annotations, primarily because we use a lot of command-line
switches (40 or more) and commandline-enabled-features as well, each of
which takes up an annotation. It's still rare (100s a day) but will
probably become worse over time as more CrashKey uses are added.

Increase kMaxNumberOfAnnotations to 400.

BUG=296821415

Change-Id: Iba7049014ee3c5ae9c45c4022600eaba50acd403
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5354336
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Ian Barkley-Yeung <iby@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
There appears to be a change in dyld in macOS 14.3 that iOS 17
accounts for, but older simulators do not. This causes the main
binary to be listed twice when iterating modules, breaking some
tests.

Bug: crbug.com/328282286

Change-Id: I71909fbc13bee6de23b10ffd92a791067f8ea909
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5353754
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
This CL make the iterators implemented by AnnotationList compliant to
the requirements imposed by the C++ standard on input iterators.

Change-Id: I263c94a97f5bcd7edd5ef4d8b65fa28b11876974
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5093147
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Arm's Pointer Authentication uses two keys for signing pointers, A-key
and B-key. Although by default Clang uses the A-key if PAC support is
enabled at compile time, this behaviour might be overridden via compiler
command line.

This CL fixes the check for the B-key being enabled. The key that shall
be used for Pointer Authentication is denoted by bits 0 (A-key) or
1 (B-key) of __ARM_FEATURE_PAC_DEFAULT. Hence, the previous way of
checking by using bits 0 and 2 does not correctly identify the B-key.

Bug: 40608466
Change-Id: Ib2f226baa12a7145fa0b6e486e49d36e6b0a3cd7
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5341090
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
After https://crrev.com/c/5375084, Chromium __libcpp_verbose_abort is
handled differently for official non-dcheck builds. This change fixes
the test expectation for release non-official builds.

Bug: 330168249
Change-Id: Iceb6d327f9e93fd366cc07abe27eefd1adf06472
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5378380
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Previously, Crashpad would only capture iOS thread context memory regions by iterating the task_threads->thread_get_state's. For Mach
exception this worked as intended.  However, for signal exceptions this
missed the registers from the actual signal context. This change
correctly captures these regions and stores them in the exception
snapshot.

Change-Id: I494e753a25c2687e61b5183ed0135f520ca8bf52
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5380505
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
@sentry

sentry Bot commented Apr 28, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 13801046

@sentry

sentry Bot commented May 7, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 13964875

Comment thread client/client_argv_handling.h
Co-authored-by: OpenAI Codex <noreply@openai.com>
@sentry

sentry Bot commented May 7, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 13967760

@sentry

sentry Bot commented May 8, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 13986410

Comment thread build/ios/setup_ios_gn.py
arg = arg.replace('*', '\\*')
else:
result.append(arg)
return ' '.join(result)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NinjaEscapeCommand silently drops arguments needing escaping

Medium Severity

In NinjaEscapeCommand, when an argument needs escaping (contains :, *, or ;), the escaped argument is never appended to result — only the else branch appends. This silently drops any arguments that contain those characters from the final command string.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ff141a8. Configure here.

@sentry

sentry Bot commented Jun 22, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 14796012

build_config = copy.copy(build_config_template)
build_config['name'] = configuration
build_config_id = self.AddObject('products', build_config)
obj['buildConfigurations'].append(build_config_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default Xcode configuration dropped

Medium Severity

When rebuilding XCConfigurationList entries, the refactor no longer sets defaultConfigurationName on each configuration list, which Xcode uses to pick the active configuration when opening the project.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6a25bba. Configure here.

Comment thread build/ios/setup_ios_gn.py
args.append(('target_cpu', self.TARGET_CPU_VALUES[self._target]))
args.append(
('target_environment', self.TARGET_ENVIRONMENT_VALUES[self._target]))
args.append(('target_platform', self.TARGET_PLATFORM_VALUES[self._target]))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignored build.arch setting

Medium Severity

setup_ios_gn.py still validates build.arch (64-bit, 32-bit, fat) in config files, but _GetGnArgs always emits a fixed target_cpu from TARGET_CPU_VALUES and never reads build.arch, so user settings for 32-bit or fat simulator builds are silently ignored.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6a25bba. Configure here.

* fix(crashpad): guard runtime IPC by owner pid

Capture the process that started the handler and reject runtime control IPC
from other processes using OS-provided peer identity.

Cover Windows named-pipe peer PIDs, Linux socket credentials, and macOS Mach
audit trailers. Pass the macOS owner PID as --client-pid during handler
startup.

* fix

* add -1 sentinel for "no owner"

* fix review findings
@sentry

sentry Bot commented Jul 10, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 15161293

Comment thread client/CMakeLists.txt
ios_handler/prune_intermediate_dumps_and_crash_reports_thread.h
simulate_crash_ios.h
)
endif()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing iOS crash handler sources

High Severity

The iOS CMake sources omit crash_handler_base_ios, crash_handler_ios / crash_handler_tvos, and upload_behavior_ios, which BUILD.gn adds and which crashpad_client_ios.cc includes. An iOS CMake build, including the new build-ios workflow job, cannot link the client.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2ab129b. Configure here.

Comment thread client/CMakeLists.txt Outdated
* fix(posix): Report spawn failures from parent process

Move spawn and exec failure logging out of the forked intermediate
process. Exit with errno from the child and reconstruct the detailed
failure message after waitpid returns in the parent.

This keeps diagnostics such as permission-denied handler startup failures
visible without running Crashpad's logging machinery in a forked child of a
multithreaded process.

Co-Authored-By: OpenAI Codex <noreply@openai.com>

* restore android

---------

Co-authored-by: OpenAI Codex <noreply@openai.com>
@sentry

sentry Bot commented Jul 16, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 15292579

The Sentry-maintained CMake metadata for `crashpad_client` still
advertised `cxx_std_14`, but the headers it installs and exposes use
C++17 library types such as `std::optional` and `std::string_view`.
That lets CMake consumers compile against those headers with an
insufficient language standard.

Update the exported usage requirement to `cxx_std_17` so the CMake
build stays in sync with the upstream Crashpad source requirements.
This does not raise sentry-native's effective Crashpad requirement:
Crashpad already sets `CMAKE_CXX_STANDARD` to `17` with
`CMAKE_CXX_STANDARD_REQUIRED` enabled, and sentry-native documents
the Crashpad backend as requiring a C++17-compatible compiler.
@sentry

sentry Bot commented Jul 17, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 15305448

ReportPendingSync() adds a report UUID to the pending queue and invokes
DoWork() directly. The upload worker can call ProcessPendingReports() at
the same time and consume that UUID before the synchronous caller does.
The synchronous call can then return while the worker is still uploading
and marking the report completed.

On Windows this makes crashpad-wait-for-upload unreliable. A process
started immediately afterward for session recovery may query Crashpad
before the stack-overflow report is completed, causing the crash session
to be missed and the integration test to fail intermittently.

Serialize ProcessPendingReports() so synchronous requests wait for any
active worker pass and do not return before the queued report has been
fully processed.
@sentry

sentry Bot commented Jul 21, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 15375497

Runtime attachment IPC can race on Windows: the handler writes a response and
then disconnects the named pipe before the client has read it. Under stress this
can abort the crashing process in Crashpad with ERROR_PIPE_NOT_CONNECTED or
"!IsSocketHandle(file)", preventing normal crash handling.
@sentry

sentry Bot commented Jul 22, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 15408543

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 5 total unresolved issues (including 4 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a40e3bd. Configure here.

Comment thread CMakeLists.txt

if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
set(LINUX TRUE)
endif()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMake IOS flag undefined

High Severity

Root CMake sets LINUX from CMAKE_SYSTEM_NAME but never defines IOS, while child lists gate macOS vs iOS sources with if(IOS) and if(APPLE AND NOT IOS). iOS configures (including CI) therefore skip iOS sources and can compile macOS client code for an iOS target.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a40e3bd. Configure here.

* feat: Add attachment update IPC

Allow clients to send replacement attachment contents to the handler with
variable-length IPC messages. The handler writes the updated bytes to the
attachment path while keeping attachment registration unchanged.

Co-Authored-By: OpenAI Codex <noreply@openai.com>

* Drop IPC on Unix - simple file operations are fast enough

* Clean up Windows

* don't wait for write - defeats the purpose

* cap payload size

* add owner checks to the new IPC methods

* fix typo

* add clarifying comments

* serialize

* file lock

* flush response

* attachments_lock_

* restrict

* restrict more

* lock

* startup vs. user attachments

* use base::span to prevent copies

* restore original removal behavior to fix integration tests

* length checks

---------

Co-authored-by: OpenAI Codex <noreply@openai.com>
@sentry

sentry Bot commented Jul 24, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 15463964

* chore(deps): bump mini_chromium

* Bump mini_chromium
@sentry

sentry Bot commented Jul 27, 2026

Copy link
Copy Markdown

🚧 Skipped: PR exceeds review size limit.

Please split into smaller PRs and re-run.
Reference ID: 15500343

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.