fix(attachments): flush scope on updating attachments

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - Add `sentry_scope_remove_fingerprint` to remove a fingerprint set on a scope, matching the global `sentry_remove_fingerprint`. ([#1932](https://github.com/getsentry/sentry-native/pull/1932))
 
+**Fixes**:
+
+- `sentry_attachment_set_filename`, `sentry_attachment_set_type`, and `sentry_attachment_set_content_type` now flush the scope, so changes applied after `sentry_attach_file`/`sentry_attach_bytes` also apply to hard-crash events instead of only to normal events. ([#1934](https://github.com/getsentry/sentry-native/pull/1934))
+
 ## 0.16.1
 
 **Features**:

examples/example.c

@@ -1217,6 +1217,15 @@ main(int argc, char **argv)
         sentry_reinstall_backend();
     }
 
+    if (has_arg(argc, argv, "attach-custom-filename")) {
+        // assuming the example / test is run directly from the cmake build
+        // directory
+        sentry_attachment_t *attachment
+            = sentry_attach_file("./CMakeCache.txt");
+        sentry_attachment_set_filename(attachment, "custom-name.log");
+        sentry_attachment_set_content_type(attachment, "application/zstd");
+    }
+
     if (has_arg(argc, argv, "flush")) {
         sentry_flush(10000);
     }

src/sentry_attachment.c

@@ -3,6 +3,7 @@
 #include "sentry_logger.h"
 #include "sentry_options.h"
 #include "sentry_path.h"
+#include "sentry_scope.h"
 #include "sentry_string.h"
 
 #include <string.h>
@@ -14,6 +15,11 @@ sentry_attachment_set_type(sentry_attachment_t *attachment, const char *type)
         attachment, type, sentry__guarded_strlen(type));
 }
 
+// The public setters can be called on an attachment that is owned by the global
+// scope, after it was added. Mutating under the scope lock and flushing on
+// unlock keeps a backend's on-disk crash state in sync with the live
+// attachment - otherwise a hard crash reports the values the attachment had
+// when it was added.
 void
 sentry_attachment_set_type_n(
     sentry_attachment_t *attachment, const char *type, size_t type_len)
@@ -22,6 +28,19 @@ sentry_attachment_set_type_n(
         return;
     }
 
+    SENTRY_WITH_SCOPE_MUT (scope) {
+        sentry__attachment_set_type_n(attachment, type, type_len);
+    }
+}
+
+void
+sentry__attachment_set_type_n(
+    sentry_attachment_t *attachment, const char *type, size_t type_len)
+{
+    if (!attachment) {
+        return;
+    }
+
     sentry_free(attachment->type);
     attachment->type
         = type && type_len > 0 ? sentry__string_clone_n(type, type_len) : NULL;
@@ -54,6 +73,20 @@ sentry_attachment_set_content_type_n(sentry_attachment_t *attachment,
         return;
     }
 
+    SENTRY_WITH_SCOPE_MUT (scope) {
+        sentry__attachment_set_content_type_n(
+            attachment, content_type, content_type_len);
+    }
+}
+
+void
+sentry__attachment_set_content_type_n(sentry_attachment_t *attachment,
+    const char *content_type, size_t content_type_len)
+{
+    if (!attachment) {
+        return;
+    }
+
     sentry_free(attachment->content_type);
     attachment->content_type
         = sentry__string_clone_n(content_type, content_type_len);
@@ -75,6 +108,19 @@ sentry_attachment_set_filename_n(
         return;
     }
 
+    SENTRY_WITH_SCOPE_MUT (scope) {
+        sentry__attachment_set_filename_n(attachment, filename, filename_len);
+    }
+}
+
+void
+sentry__attachment_set_filename_n(
+    sentry_attachment_t *attachment, const char *filename, size_t filename_len)
+{
+    if (!attachment) {
+        return;
+    }
+
     sentry__path_free(attachment->filename);
     attachment->filename = sentry__path_from_str_n(filename, filename_len);
 }
@@ -96,6 +142,19 @@ sentry_attachment_set_filenamew_n(sentry_attachment_t *attachment,
         return;
     }
 
+    SENTRY_WITH_SCOPE_MUT (scope) {
+        sentry__attachment_set_filenamew_n(attachment, filename, filename_len);
+    }
+}
+
+void
+sentry__attachment_set_filenamew_n(sentry_attachment_t *attachment,
+    const wchar_t *filename, size_t filename_len)
+{
+    if (!attachment) {
+        return;
+    }
+
     sentry__path_free(attachment->filename);
     attachment->filename = sentry__path_from_wstr_n(filename, filename_len);
 }
@@ -255,9 +314,12 @@ sentry__attachments_add_path(sentry_attachment_t **attachments_ptr,
     if (!attachment) {
         return NULL;
     }
-    sentry_attachment_set_type(attachment, attachment_type);
+    // these lists are not owned by the global scope, so no flush is needed
+    sentry__attachment_set_type_n(
+        attachment, attachment_type, sentry__guarded_strlen(attachment_type));
     if (content_type || !attachment->content_type) {
-        sentry_attachment_set_content_type(attachment, content_type);
+        sentry__attachment_set_content_type_n(
+            attachment, content_type, sentry__guarded_strlen(content_type));
     }
     return sentry__attachments_add(attachments_ptr, attachment);
 }

src/sentry_attachment.h

@@ -34,6 +34,26 @@ struct sentry_attachment_s {
     sentry_attachment_t *next; // Linked list pointer
 };
 
+/**
+ * These mutate the `attachment` without locking or flushing the scope, for
+ * attachments that are not owned by the global scope (options, hints, and
+ * backend-internal lists).
+ *
+ * The public `sentry_attachment_set_*` counterparts must be used for anything
+ * that can be scope-owned: they mutate under the scope lock and flush the
+ * backend on unlock, so a backend's on-disk crash state stays in sync.
+ */
+void sentry__attachment_set_type_n(
+    sentry_attachment_t *attachment, const char *type, size_t type_len);
+void sentry__attachment_set_content_type_n(sentry_attachment_t *attachment,
+    const char *content_type, size_t content_type_len);
+void sentry__attachment_set_filename_n(
+    sentry_attachment_t *attachment, const char *filename, size_t filename_len);
+#ifdef SENTRY_PLATFORM_WINDOWS
+void sentry__attachment_set_filenamew_n(sentry_attachment_t *attachment,
+    const wchar_t *filename, size_t filename_len);
+#endif
+
 /**
  * Returns the size in bytes of the attachment's data (buffer length or file
  * size).

tests/test_integration_native.py

@@ -5,6 +5,7 @@
 multi-thread capture, and FPU/SIMD register capture on all platforms.
 """
 
+import json
 import os
 import subprocess
 import sys
@@ -32,6 +33,7 @@
     assert_replay_envelope,
     assert_session,
     is_valid_hex,
+    wait_for,
     wait_for_file,
     assert_user_feedback,
 )
@@ -285,6 +287,58 @@ def test_native_overflow_breadcrumbs(cmake, httpserver, crash_mode):
     assert any(b.get("message") == "100" for b in breadcrumbs)
 
 
+def test_native_attachment_manifest_is_current(cmake, httpserver):
+    """The attachment manifest must reflect the live attachment state (#1933).
+
+    The daemon builds hard-crash envelopes from `<run>/__sentry-attachments`,
+    which the backend only rewrites on a scope flush. `sentry_attach_file`
+    flushes, but `sentry_attachment_set_filename`/`_set_content_type` do not, so
+    the manifest keeps the physical basename and the crash event reports
+    `CMakeCache.txt` instead of `custom-name.log`.
+
+    Asserting on the manifest rather than on a crash envelope keeps this
+    deterministic: the in-process crash handler happens to flush the scope (via
+    `sentry__trace_finish`), which repairs the manifest before the daemon reads
+    it, but out-of-process paths such as WER never run that handler.
+    """
+    tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"})
+
+    exe = tmp_path / (
+        "sentry_example.exe" if sys.platform == "win32" else "sentry_example"
+    )
+    child = subprocess.Popen(
+        [str(exe), "log", "attach-custom-filename", "sleep"],
+        cwd=tmp_path,
+        env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)),
+    )
+    db_dir = tmp_path / ".sentry-native"
+
+    def read_manifest():
+        # `sentry_attach_file` writes the manifest, the setters are expected to
+        # rewrite it, so poll instead of reading the first version we see
+        paths = list(db_dir.glob("*.run/__sentry-attachments"))
+        if not paths:
+            return None
+        try:
+            return json.loads(paths[0].read_text())
+        except (OSError, json.JSONDecodeError):
+            return None
+
+    try:
+        renamed = wait_for(
+            lambda: (read_manifest() or [{}])[0].get("filename") == "custom-name.log"
+        )
+        manifest = read_manifest()
+    finally:
+        child.terminate()
+        child.wait()
+
+    assert renamed, f"attachment manifest kept the physical filename: {manifest}"
+    assert len(manifest) == 1
+    assert manifest[0]["content_type"] == "application/zstd"
+    assert manifest[0]["path"].endswith("CMakeCache.txt")
+
+
 def test_native_session_tracking(cmake, httpserver):
     """Test that sessions are tracked correctly with crashes"""
     tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"})