fix: Various fixes (#3308)

This PR contains the following commits, which are ported from a private
repo

-
[`e344276`](e344276):
changelog update
-
[`f585427`](f585427):
getsentry/sentry-cli-security-fixes#13
-
[`29fe087`](29fe087):
getsentry/sentry-cli-security-fixes#22
-
[`36d24c2`](36d24c2):
getsentry/sentry-cli-security-fixes#18
-
[`dc2c943`](dc2c943):
getsentry/sentry-cli-security-fixes#19
-
[`1e31ca8`](1e31ca8):
getsentry/sentry-cli-security-fixes#21
-
[`769b8ce`](769b8ce):
getsentry/sentry-cli-security-fixes#11

#skip-changelog

Author: szokeasaurusrex

.zed/settings.json

@@ -3,12 +3,10 @@
     "rust-analyzer": {
       "initialization_options": {
         "check": {
-          "command": "clippy"
+          "command": "clippy",
         },
-        "cargo": {
-          "allFeatures": true
-        }
-      }
-    }
-  }
+        "cargo": {},
+      },
+    },
+  },
 }

CHANGELOG.md

@@ -2,6 +2,15 @@
 
 ## Unreleased
 
+### Security Fixes
+
+- **Behavior-breaking**: Disable Xcode `Info.plist` preprocessing by default to avoid passing project-controlled compiler settings to `cc` during release auto-discovery. This affects `sentry-cli releases propose-version`, `sentry-cli send-event` and `sentry-cli bash-hook --send-event` release inference, and `sentry-cli react-native xcode` auto-release detection. Use `--allow-xcode-infoplist-preprocessing` only for trusted projects that require preprocessing.
+- Ensure restrictive file permissions maintained when `sentry-cli login` updates existing config files.
+- Disable TLS verification only when `http.verify_ssl` is set to `false`, case-insensitively.
+- Shell-escape generated `bash-hook` arguments, including paths, tags, release names, and the CLI path.
+- Stop sending environment variables in `sentry-cli bash-hook` events.
+- Verify the downloaded binary checksum before replacing the current executable in `sentry-cli update`.
+
 ### Performance
 
 - (snapshots) Skip uploading images that already exist in objectstore by batch-checking with HEAD requests first ([#3305](https://github.com/getsentry/sentry-cli/pull/3305))

src/api/mod.rs

@@ -11,6 +11,7 @@ mod encoding;
 mod errors;
 mod pagination;
 mod serialization;
+mod updating;
 
 use std::borrow::Cow;
 use std::cell::RefCell;
@@ -57,6 +58,7 @@ use encoding::{PathArg, QueryArg};
 use errors::{ApiError, ApiErrorKind, ApiResult, SentryError};
 
 pub use self::data_types::*;
+pub use crate::api::updating::ReleaseRegistryFile;
 
 lazy_static! {
     static ref API: Mutex<Option<Arc<Api>>> = Mutex::new(None);
@@ -335,13 +337,13 @@ impl Api {
 
         if resp.status() == 200 {
             let info: RegistryRelease = resp.convert()?;
-            for (filename, _download_url) in info.file_urls {
+            for (filename, _download_url) in info.files {
                 info!("Found asset {filename}");
                 if filename == ref_name {
                     return Ok(Some(SentryCliRelease {
                         version: info.version,
                         #[cfg(not(feature = "managed"))]
-                        download_url: _download_url,
+                        download_info: _download_url,
                     }));
                 }
             }
@@ -1729,17 +1731,17 @@ pub struct ReleaseCommit {
     pub id: String,
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Deserialize)]
 struct RegistryRelease {
     version: String,
-    file_urls: HashMap<String, String>,
+    files: HashMap<String, ReleaseRegistryFile>,
 }
 
 /// Information about sentry CLI releases
 pub struct SentryCliRelease {
     pub version: String,
     #[cfg(not(feature = "managed"))]
-    pub download_url: String,
+    pub download_info: ReleaseRegistryFile,
 }
 
 #[derive(Debug, Deserialize, Default)]

src/api/updating.rs

@@ -0,0 +1,80 @@
+//! Types for updating functionality.
+
+use std::ops::Deref;
+use std::str::FromStr;
+
+use anyhow::{Context as _, Error};
+use serde::de::Error as _;
+use serde::{Deserialize, Deserializer};
+
+/// A SHA-256 sum in hexadecimal representation is 64 characters long.
+const SHA256_SUM_HEX_LENGTH: usize = 64;
+
+#[derive(Debug)]
+pub struct Sha256Sum([u8; 32]);
+
+#[derive(Debug, Deserialize)]
+pub struct ReleaseRegistryFile {
+    pub url: String,
+    #[serde(rename = "checksums")]
+    #[serde(deserialize_with = "deserialize_checksums")]
+    pub checksum: Sha256Sum,
+}
+
+fn deserialize_checksums<'de, D>(deserializer: D) -> Result<Sha256Sum, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    #[derive(Deserialize)]
+    #[serde(rename_all = "kebab-case")]
+    struct RawChecksumsMapping {
+        sha256_hex: String,
+    }
+
+    let RawChecksumsMapping { sha256_hex } = RawChecksumsMapping::deserialize(deserializer)?;
+    sha256_hex.parse().map_err(D::Error::custom)
+}
+
+impl FromStr for Sha256Sum {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.len() != SHA256_SUM_HEX_LENGTH {
+            anyhow::bail!(
+                "cannot parse SHA-256: expected a {SHA256_SUM_HEX_LENGTH}-character long string"
+            );
+        }
+
+        let mut bytes = [0u8; 32];
+
+        bytes
+            .iter_mut()
+            .zip(s.as_bytes().chunks(2))
+            .map(|(byte, hex_byte)| {
+                let hex_str = str::from_utf8(hex_byte)?;
+                *byte = u8::from_str_radix(hex_str, 16)?;
+                Ok::<_, Self::Err>(())
+            })
+            .map(|result| result.context("cannot parse SHA-256: not a valid hex string"))
+            .collect::<Result<Vec<()>, _>>()?;
+
+        Ok(Sha256Sum(bytes))
+    }
+}
+
+impl Deref for Sha256Sum {
+    type Target = [u8; 32];
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<Rhs> PartialEq<Rhs> for Sha256Sum
+where
+    Rhs: Deref<Target = [u8]>,
+{
+    fn eq(&self, other: &Rhs) -> bool {
+        self.0 == **other
+    }
+}

src/bashsupport.sh

@@ -1,5 +1,5 @@
-_SENTRY_TRACEBACK_FILE="___SENTRY_TRACEBACK_FILE___"
-_SENTRY_LOG_FILE="___SENTRY_LOG_FILE___"
+_SENTRY_TRACEBACK_FILE=___SENTRY_TRACEBACK_FILE___
+_SENTRY_LOG_FILE=___SENTRY_LOG_FILE___
 
 if [ "${SENTRY_CLI_NO_EXIT_TRAP-0}" != 1 ]; then
   trap _sentry_exit_trap EXIT
@@ -32,7 +32,7 @@ _sentry_err_trap() {
   echo "@exit_code:${_exit_code}" >> "$_SENTRY_TRACEBACK_FILE"
 
   : >> "$_SENTRY_LOG_FILE"
-  export SENTRY_LAST_EVENT=$(___SENTRY_CLI___ bash-hook --send-event --traceback "$_SENTRY_TRACEBACK_FILE" ___SENTRY_TAGS___ ___SENTRY_RELEASE___ --log "$_SENTRY_LOG_FILE" ___SENTRY_NO_ENVIRON___)
+  export SENTRY_LAST_EVENT=$(___SENTRY_CLI___ bash-hook --send-event --traceback "$_SENTRY_TRACEBACK_FILE" ___SENTRY_TAGS___ ___SENTRY_RELEASE___ ___SENTRY_ALLOW_XCODE_INFOPLIST_PREPROCESSING___ --log "$_SENTRY_LOG_FILE")
   rm -f "$_SENTRY_TRACEBACK_FILE" "$_SENTRY_LOG_FILE"
 }
 

src/commands/bash_hook.rs

@@ -11,11 +11,12 @@ use anyhow::{format_err, Result};
 use clap::{builder::ArgPredicate, Arg, ArgAction, ArgMatches, Command};
 use lazy_static::lazy_static;
 use regex::Regex;
-use sentry::protocol::{Event, Exception, Frame, Stacktrace, User, Value};
+use sentry::protocol::{Event, Exception, Frame, Stacktrace, User};
 use uuid::Uuid;
 
 use crate::commands::send_event;
 use crate::config::Config;
+use crate::utils::args::allow_xcode_infoplist_preprocessing_arg;
 use crate::utils::event::{attach_logfile, get_sdk_info};
 use crate::utils::releases::detect_release_name;
 
@@ -40,15 +41,17 @@ pub fn make_command(command: Command) -> Command {
         .arg(
             Arg::new("no_environ")
                 .long("no-environ")
+                .hide(true)
                 .action(ArgAction::SetTrue)
-                .help("Do not send environment variables along"),
+                .help("No-op, as we never send envrionment variables."),
         )
         .arg(
             Arg::new("cli")
                 .long("cli")
                 .value_name("CMD")
                 .help("Explicitly set/override the sentry-cli command"),
         )
+        .arg(allow_xcode_infoplist_preprocessing_arg())
         .arg(
             Arg::new("send_event")
                 .long("send-event")
@@ -87,13 +90,15 @@ fn send_event(
     logfile: &str,
     tags: &[&String],
     release: Option<String>,
-    environ: bool,
+    allow_xcode_infoplist_preprocessing: bool,
 ) -> Result<()> {
     let config = Config::current();
 
     let mut event = Event {
         environment: config.get_environment().map(Into::into),
-        release: release.or(detect_release_name().ok()).map(Into::into),
+        release: release
+            .or(detect_release_name(allow_xcode_infoplist_preprocessing).ok())
+            .map(Into::into),
         sdk: Some(get_sdk_info()),
         user: whoami::fallible::username().ok().map(|n| User {
             username: Some(n),
@@ -112,13 +117,6 @@ fn send_event(
         event.tags.insert(key.into(), value.into());
     }
 
-    if environ {
-        event.extra.insert(
-            "environ".into(),
-            Value::Object(env::vars().map(|(k, v)| (k, Value::String(v))).collect()),
-        );
-    }
-
     let mut cmd = "unknown".to_owned();
     let mut exit_code = 1;
     let mut frames = vec![];
@@ -208,6 +206,10 @@ fn send_event(
     Ok(())
 }
 
+fn shell_quote(value: &str) -> String {
+    format!("'{}'", value.replace('\'', r"'\''"))
+}
+
 pub fn execute(matches: &ArgMatches) -> Result<()> {
     let release = Config::current().get_release(matches).ok();
 
@@ -222,7 +224,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
             matches.get_one::<String>("log").unwrap(),
             &tags,
             release,
-            !matches.get_flag("no_environ"),
+            matches.get_flag("allow_xcode_infoplist_preprocessing"),
         );
     }
 
@@ -235,42 +237,45 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
     let mut script = BASH_SCRIPT
         .replace(
             "___SENTRY_TRACEBACK_FILE___",
-            &traceback.display().to_string(),
+            &shell_quote(&traceback.display().to_string()),
         )
-        .replace("___SENTRY_LOG_FILE___", &log.display().to_string());
+        .replace(
+            "___SENTRY_LOG_FILE___",
+            &shell_quote(&log.display().to_string()),
+        );
 
     script = script.replace(
         " ___SENTRY_TAGS___",
         &tags
             .iter()
-            .map(|tag| format!(" --tag \"{tag}\""))
+            .map(|tag| format!(" --tag {}", shell_quote(tag)))
             .collect::<Vec<_>>()
             .join(""),
     );
 
     script = match release {
         Some(release) => script.replace(
             " ___SENTRY_RELEASE___",
-            format!(" --release \"{release}\"").as_str(),
+            format!(" --release {}", shell_quote(&release)).as_str(),
         ),
         None => script.replace(" ___SENTRY_RELEASE___", ""),
     };
 
     script = script.replace(
         "___SENTRY_CLI___",
-        matches
-            .get_one::<String>("cli")
-            .map_or_else(
-                || env::current_exe().unwrap().display().to_string(),
-                String::clone,
-            )
-            .as_str(),
+        &shell_quote(&matches.get_one::<String>("cli").map_or_else(
+            || env::current_exe().unwrap().display().to_string(),
+            String::clone,
+        )),
     );
 
-    if matches.get_flag("no_environ") {
-        script = script.replace("___SENTRY_NO_ENVIRON___", "--no-environ");
+    if matches.get_flag("allow_xcode_infoplist_preprocessing") {
+        script = script.replace(
+            " ___SENTRY_ALLOW_XCODE_INFOPLIST_PREPROCESSING___",
+            " --allow-xcode-infoplist-preprocessing",
+        );
     } else {
-        script = script.replace("___SENTRY_NO_ENVIRON___", "");
+        script = script.replace(" ___SENTRY_ALLOW_XCODE_INFOPLIST_PREPROCESSING___", "");
     }
 
     if !matches.get_flag("no_exit") {
@@ -279,3 +284,16 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
     println!("{script}");
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::shell_quote;
+
+    #[test]
+    fn shell_quote_handles_special_characters() {
+        assert_eq!(
+            shell_quote("it's $(unsafe); && ok"),
+            "'it'\\''s $(unsafe); && ok'"
+        );
+    }
+}

src/commands/react_native/xcode.rs

@@ -18,7 +18,9 @@ use serde_json::Value;
 use crate::api::Api;
 use crate::config::Config;
 use crate::constants::DEFAULT_MAX_WAIT;
-use crate::utils::args::{validate_distribution, ArgExt as _};
+use crate::utils::args::{
+    allow_xcode_infoplist_preprocessing_arg, validate_distribution, ArgExt as _,
+};
 use crate::utils::file_search::ReleaseFileSearch;
 use crate::utils::file_upload::UploadContext;
 use crate::utils::fs::TempFile;
@@ -122,6 +124,7 @@ pub fn make_command(command: Command) -> Command {
                      but at most for the given number of seconds.",
                 ),
         )
+        .arg(allow_xcode_infoplist_preprocessing_arg())
         .arg(
             Arg::new("no_auto_release")
                 .long("no-auto-release")
@@ -358,7 +361,9 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
             (Err(_), Err(_)) => {
                 // Neither environment variable is present, attempt to parse Info.plist
                 info!("Parsing Info.plist");
-                match InfoPlist::discover_from_env() {
+                match InfoPlist::discover_from_env(
+                    matches.get_flag("allow_xcode_infoplist_preprocessing"),
+                ) {
                     Ok(Some(plist)) => {
                         // Successfully discovered and parsed Info.plist
                         let dist_string = plist.build().to_owned();

src/commands/releases/propose_version.rs

@@ -1,13 +1,19 @@
 use anyhow::Result;
 use clap::{ArgMatches, Command};
 
+use crate::utils::args::allow_xcode_infoplist_preprocessing_arg;
 use crate::utils::releases::detect_release_name;
 
 pub fn make_command(command: Command) -> Command {
-    command.about("Propose a version name for a new release.")
+    command
+        .about("Propose a version name for a new release.")
+        .arg(allow_xcode_infoplist_preprocessing_arg())
 }
 
-pub fn execute(_matches: &ArgMatches) -> Result<()> {
-    println!("{}", detect_release_name()?);
+pub fn execute(matches: &ArgMatches) -> Result<()> {
+    println!(
+        "{}",
+        detect_release_name(matches.get_flag("allow_xcode_infoplist_preprocessing"))?
+    );
     Ok(())
 }