Redact string panics (#51248)

Conrad Irwin created 

String panics are a non-trivial percentage of the crashes we see at Zed,
and
doubly unfortunately they may incidentally include the contents of a
user's
buffer.

Although this hasn't happened yet (to my knowledge), I don't want to be
in the position of having received sensitive information this way.

See also https://github.com/rust-lang/rust/pull/153677

Release Notes:

- N/A

Change summary

Cargo.lock                                |  1 
crates/crashes/src/crashes.rs             | 28 +++++++++++
crates/feature_flags/Cargo.toml           |  1 
crates/feature_flags/src/feature_flags.rs | 61 ------------------------
crates/zed/src/zed.rs                     | 33 +++++++------
5 files changed, 46 insertions(+), 78 deletions(-)

Detailed changes

Cargo.lock 🔗 

@@ -6212,7 +6212,6 @@ dependencies = [
 name = "feature_flags"
 version = "0.1.0"
 dependencies = [
- "futures 0.3.31",
  "gpui",
 ]

crates/crashes/src/crashes.rs 🔗 

@@ -350,8 +350,34 @@ impl minidumper::ServerHandler for CrashServer {
     }
 }
 
+/// Rust's string-slicing panics embed the user's string content in the message,
+/// e.g. "byte index 4 is out of bounds of `a`". Strip that suffix so we
+/// don't upload arbitrary user text in crash reports.
+fn strip_user_string_from_panic(message: &str) -> String {
+    const STRING_PANIC_PREFIXES: &[&str] = &[
+        // Older rustc (pre-1.95):
+        "byte index ",
+        "begin <= end (",
+        // Newer rustc (1.95+):
+        // https://github.com/rust-lang/rust/pull/145024
+        "start byte index ",
+        "end byte index ",
+        "begin > end (",
+    ];
+
+    if (message.ends_with('`') || message.ends_with("`[...]"))
+        && STRING_PANIC_PREFIXES
+            .iter()
+            .any(|prefix| message.starts_with(prefix))
+        && let Some(open) = message.find('`')
+    {
+        return format!("{} `<redacted>`", &message[..open]);
+    }
+    message.to_owned()
+}
+
 pub fn panic_hook(info: &PanicHookInfo) {
-    let message = info.payload_as_str().unwrap_or("Box<Any>").to_owned();
+    let message = strip_user_string_from_panic(info.payload_as_str().unwrap_or("Box<Any>"));
 
     let span = info
         .location()

crates/feature_flags/src/feature_flags.rs 🔗 

@@ -3,12 +3,8 @@ mod flags;
 use std::cell::RefCell;
 use std::rc::Rc;
 use std::sync::LazyLock;
-use std::time::Duration;
-use std::{future::Future, pin::Pin, task::Poll};
 
-use futures::channel::oneshot;
-use futures::{FutureExt, select_biased};
-use gpui::{App, Context, Global, Subscription, Task, Window};
+use gpui::{App, Context, Global, Subscription, Window};
 
 pub use flags::*;
 
@@ -122,11 +118,6 @@ pub struct OnFlagsReady {
 }
 
 pub trait FeatureFlagAppExt {
-    fn wait_for_flag<T: FeatureFlag>(&mut self) -> WaitForFlag;
-
-    /// Waits for the specified feature flag to resolve, up to the given timeout.
-    fn wait_for_flag_or_timeout<T: FeatureFlag>(&mut self, timeout: Duration) -> Task<bool>;
-
     fn update_flags(&mut self, staff: bool, flags: Vec<String>);
     fn set_staff(&mut self, staff: bool);
     fn has_flag<T: FeatureFlag>(&self) -> bool;
@@ -192,54 +183,4 @@ impl FeatureFlagAppExt for App {
             callback(feature_flags.has_flag::<T>(), cx);
         })
     }
-
-    fn wait_for_flag<T: FeatureFlag>(&mut self) -> WaitForFlag {
-        let (tx, rx) = oneshot::channel::<bool>();
-        let mut tx = Some(tx);
-        let subscription: Option<Subscription>;
-
-        match self.try_global::<FeatureFlags>() {
-            Some(feature_flags) => {
-                subscription = None;
-                tx.take().unwrap().send(feature_flags.has_flag::<T>()).ok();
-            }
-            None => {
-                subscription = Some(self.observe_global::<FeatureFlags>(move |cx| {
-                    let feature_flags = cx.global::<FeatureFlags>();
-                    if let Some(tx) = tx.take() {
-                        tx.send(feature_flags.has_flag::<T>()).ok();
-                    }
-                }));
-            }
-        }
-
-        WaitForFlag(rx, subscription)
-    }
-
-    fn wait_for_flag_or_timeout<T: FeatureFlag>(&mut self, timeout: Duration) -> Task<bool> {
-        let wait_for_flag = self.wait_for_flag::<T>();
-
-        self.spawn(async move |cx| {
-            let mut wait_for_flag = wait_for_flag.fuse();
-            let mut timeout = FutureExt::fuse(cx.background_executor().timer(timeout));
-
-            select_biased! {
-                is_enabled = wait_for_flag => is_enabled,
-                _ = timeout => false,
-            }
-        })
-    }
-}
-
-pub struct WaitForFlag(oneshot::Receiver<bool>, Option<Subscription>);
-
-impl Future for WaitForFlag {
-    type Output = bool;
-
-    fn poll(mut self: Pin<&mut Self>, cx: &mut core::task::Context<'_>) -> Poll<Self::Output> {
-        self.0.poll_unpin(cx).map(|result| {
-            self.1.take();
-            result.unwrap_or(false)
-        })
-    }
 }

crates/zed/src/zed.rs 🔗 

@@ -163,21 +163,24 @@ pub fn init(cx: &mut App) {
     cx.on_action(quit);
 
     cx.on_action(|_: &RestoreBanner, cx| title_bar::restore_banner(cx));
-    let flag = cx.wait_for_flag::<PanicFeatureFlag>();
-    cx.spawn(async |cx| {
-        if cx.update(|cx| ReleaseChannel::global(cx) == ReleaseChannel::Dev) || flag.await {
-            cx.update(|cx| {
-                cx.on_action(|_: &TestPanic, _| panic!("Ran the TestPanic action"))
-                    .on_action(|_: &TestCrash, _| {
-                        unsafe extern "C" {
-                            fn puts(s: *const i8);
-                        }
-                        unsafe {
-                            puts(0xabad1d3a as *const i8);
-                        }
-                    });
-            });
-        };
+
+    cx.observe_flag::<PanicFeatureFlag, _>({
+        let mut added = false;
+        move |enabled, cx| {
+            if added || !enabled {
+                return;
+            }
+            added = true;
+            cx.on_action(|_: &TestPanic, _| panic!("Ran the TestPanic action"))
+                .on_action(|_: &TestCrash, _| {
+                    unsafe extern "C" {
+                        fn puts(s: *const i8);
+                    }
+                    unsafe {
+                        puts(0xabad1d3a as *const i8);
+                    }
+                });
+        }
     })
     .detach();
     cx.on_action(|_: &OpenLog, cx| {