Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 45 additions & 10 deletions src/commands/mobile_app/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,28 @@ use crate::config::Config;
use crate::utils::args::ArgExt;
use crate::utils::chunks::{upload_chunks, Chunk, ASSEMBLE_POLL_INTERVAL};
use crate::utils::fs::get_sha1_checksums;
#[cfg(target_os = "macos")]
use crate::utils::fs::TempDir;
use crate::utils::fs::TempFile;
#[cfg(target_os = "macos")]
use crate::utils::mobile_app::handle_asset_catalogs;
use crate::utils::mobile_app::{handle_asset_catalogs, ipa_to_xcarchive, is_ipa_file};
use crate::utils::mobile_app::{is_aab_file, is_apk_file, is_apple_app, is_zip_file};
use crate::utils::progress::ProgressBar;
use crate::utils::vcs;

pub fn make_command(command: Command) -> Command {
#[cfg(target_os = "macos")]
const HELP_TEXT: &str = "The path to the mobile app files to upload. Supported files include Apk, Aab, XCArchive, and IPA.";
#[cfg(not(target_os = "macos"))]
const HELP_TEXT: &str = "The path to the mobile app files to upload. Supported files include Apk, Aab, and XCArchive.";
command
.about("[EXPERIMENTAL] Upload mobile app files to a project.")
.org_arg()
.project_arg(false)
.arg(
Arg::new("paths")
.value_name("PATH")
.help("The path to the mobile app files to upload. Supported files include Apk, Aab or XCArchive.")
.help(HELP_TEXT)
.num_args(1..)
.action(ArgAction::Append)
.required(true),
Expand Down Expand Up @@ -95,12 +101,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {

let normalized_zip = if path.is_file() {
debug!("Normalizing file: {}", path.display());
normalize_file(path, &byteview).with_context(|| {
format!(
"Failed to generate uploadable bundle for file {}",
path.display()
)
})?
handle_file(path, &byteview)?
} else if path.is_dir() {
debug!("Normalizing directory: {}", path.display());
normalize_directory(path).with_context(|| {
Expand Down Expand Up @@ -178,6 +179,25 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
Ok(())
}

fn handle_file(path: &Path, byteview: &ByteView) -> Result<TempFile> {
// Handle IPA files by converting them to XCArchive
#[cfg(target_os = "macos")]
if is_zip_file(byteview) && is_ipa_file(byteview)? {
debug!("Converting IPA file to XCArchive structure");
let temp_dir = TempDir::create()?;
return ipa_to_xcarchive(path, byteview, &temp_dir)
.and_then(|path| normalize_directory(&path))
.with_context(|| format!("Failed to process IPA file {}", path.display()));
}

normalize_file(path, byteview).with_context(|| {
format!(
"Failed to generate uploadable bundle for file {}",
path.display()
)
})
}

fn validate_is_mobile_app(path: &Path, bytes: &[u8]) -> Result<()> {
debug!("Validating mobile app format for: {}", path.display());

Expand All @@ -186,9 +206,13 @@ fn validate_is_mobile_app(path: &Path, bytes: &[u8]) -> Result<()> {
return Ok(());
}

// Check if the file is a zip file (then AAB or APK)
// Check if the file is a zip file (then AAB, APK, or IPA)
if is_zip_file(bytes) {
#[cfg(target_os = "macos")]
debug!("File is a zip, checking for AAB/APK/IPA format");
#[cfg(not(target_os = "macos"))]
debug!("File is a zip, checking for AAB/APK format");

if is_aab_file(bytes)? {
debug!("Detected AAB file");
return Ok(());
Expand All @@ -198,11 +222,22 @@ fn validate_is_mobile_app(path: &Path, bytes: &[u8]) -> Result<()> {
debug!("Detected APK file");
return Ok(());
}

#[cfg(target_os = "macos")]
if is_ipa_file(bytes)? {
debug!("Detected IPA file");
return Ok(());
}
}

debug!("File format validation failed");
#[cfg(target_os = "macos")]
let format_list = "APK, AAB, XCArchive, or IPA";
#[cfg(not(target_os = "macos"))]
let format_list = "APK, AAB, or XCArchive";
Comment on lines +234 to +237
Copy link
Member

Choose a reason for hiding this comment

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

As we have to conditionally print this list in several places, it might eventually make sense to extract this into a constant. I don't think we have to do it now necessarily but could make the code more easily maintainable, especially if we add more file formats in the future


Err(anyhow!(
"File is not a recognized mobile app format (APK, AAB, or XCArchive): {}",
"File is not a recognized mobile app format ({format_list}): {}",
path.display()
))
}
Expand Down
131 changes: 130 additions & 1 deletion src/utils/mobile_app/apple.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
use std::path::{Path, PathBuf};
use anyhow::{anyhow, Result};
use log::debug;
use regex::Regex;
use std::{
path::{Path, PathBuf},
sync::LazyLock,
};

use crate::utils::fs::TempDir;
use apple_catalog_parsing;
use std::io::Cursor;
use walkdir::WalkDir;
use zip::ZipArchive;

pub fn handle_asset_catalogs(path: &Path) {
// Find all asset catalogs
Expand All @@ -27,3 +36,123 @@ fn find_car_files(root: &Path) -> Vec<PathBuf> {
.map(|e| e.into_path())
.collect()
}

/// Converts an IPA file to an XCArchive directory structure. The provided IPA must be a valid IPA file.
///
/// # Format Overview
///
/// ## IPA (iOS App Store Package)
/// An IPA file is a compressed archive containing an iOS app ready for distribution.
/// It has the following structure:
/// ```
/// MyApp.ipa
/// └── Payload/
/// └── MyApp.app/
/// ├── Info.plist
/// ├── MyApp (executable)
/// ├── Assets.car
/// └── ... (other app resources)
/// ```
///
/// ## XCArchive (Xcode Archive)
/// An XCArchive is a directory structure created by Xcode when archiving an app for distribution.
/// It has the following structure:
/// ```
/// MyApp.xcarchive/
/// ├── Info.plist
/// ├── Products/
/// │ └── Applications/
/// │ └── MyApp.app/
/// │ ├── Info.plist
/// │ ├── MyApp (executable)
/// │ ├── Assets.car
/// │ └── ... (other app resources)
/// └── ... (other archive metadata)
/// ```
pub fn ipa_to_xcarchive(ipa_path: &Path, ipa_bytes: &[u8], temp_dir: &TempDir) -> Result<PathBuf> {
debug!(
"Converting IPA to XCArchive structure: {}",
ipa_path.display()
);

let xcarchive_dir = temp_dir.path().join("archive.xcarchive");
let products_dir = xcarchive_dir.join("Products");
let applications_dir = products_dir.join("Applications");

debug!("Creating XCArchive directory structure");
std::fs::create_dir_all(&applications_dir)?;

// Extract IPA file
let cursor = Cursor::new(ipa_bytes);
let mut ipa_archive = ZipArchive::new(cursor)?;

let app_name = extract_app_name_from_ipa(&ipa_archive)?.to_owned();

// Extract all files from the archive
for i in 0..ipa_archive.len() {
let mut file = ipa_archive.by_index(i)?;

if let Some(name) = file.enclosed_name() {
if let Ok(stripped) = name.strip_prefix("Payload/") {
if !file.is_dir() {
// Create the file path in the XCArchive structure
let target_path = applications_dir.join(stripped);

// Create parent directories if necessary
if let Some(parent) = target_path.parent() {
std::fs::create_dir_all(parent)?;
}

// Extract file
let mut target_file = std::fs::File::create(&target_path)?;
std::io::copy(&mut file, &mut target_file)?;
}
}
}
}

// Create Info.plist for XCArchive
let info_plist_path = xcarchive_dir.join("Info.plist");

let info_plist_content = format!(
r#"<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>ApplicationProperties</key>
<dict>
<key>ApplicationPath</key>
<string>Applications/{app_name}.app</string>
</dict>
<key>ArchiveVersion</key>
<integer>1</integer>
</dict>
</plist>"#
);

std::fs::write(&info_plist_path, info_plist_content)?;

debug!(
"Created XCArchive Info.plist at: {}",
info_plist_path.display()
);
Ok(xcarchive_dir)
}

static PATTERN: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"^Payload/([^/]+)\.app/Info\.plist$").expect("regex is valid"));

fn extract_app_name_from_ipa<'a>(archive: &'a ZipArchive<Cursor<&[u8]>>) -> Result<&'a str> {
let matches = archive
.file_names()
.filter_map(|name| PATTERN.captures(name))
.map(|c| c.get(1).expect("group 1 must be present").as_str())
.take(2) // If there are ≥2 matches, we already know the IPA is invalid
.collect::<Vec<_>>();

if let &[app_name] = matches.as_slice() {
Ok(app_name)
} else {
Err(anyhow!("IPA did not contain exactly one .app."))
}
}
4 changes: 3 additions & 1 deletion src/utils/mobile_app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,7 @@ mod apple;
mod validation;

#[cfg(target_os = "macos")]
pub use self::apple::handle_asset_catalogs;
pub use self::apple::{handle_asset_catalogs, ipa_to_xcarchive};
#[cfg(target_os = "macos")]
pub use self::validation::is_ipa_file;
pub use self::validation::{is_aab_file, is_apk_file, is_apple_app, is_zip_file};
14 changes: 14 additions & 0 deletions src/utils/mobile_app/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ pub fn is_aab_file(bytes: &[u8]) -> Result<bool> {
Ok(has_bundle_config && has_base_manifest)
}

#[cfg(target_os = "macos")]
pub fn is_ipa_file(bytes: &[u8]) -> Result<bool> {
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering, why do we have this validation code in the first place? Why not just try to parse the file as all the different file types directly, then if we fail to, we assume that the file is invalid?

Having the separate validation logic seems like it is leading to duplicated code paths. We already check this structure in ipa.rs, in the extract_app_name_from_ipa method.

Fixing this (especially for the other file types) is of course out of scope for this PR, but hoping you can shed some light on this architectural decision.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed on a call, it's mostly because this was ported from our typescript code, we probably could refactor this and make it at least just file extension checking

let cursor = std::io::Cursor::new(bytes);
let archive = zip::ZipArchive::new(cursor)?;

let is_ipa = archive.file_names().any(|name| {
name.starts_with("Payload/")
&& name.ends_with(".app/Info.plist")
&& name.matches('/').count() == 2
});
Comment on lines +45 to +49
Copy link
Member

Choose a reason for hiding this comment

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

we could reuse the regex here, but this is also fine


Ok(is_ipa)
}

pub fn is_xcarchive_directory<P>(path: P) -> bool
where
P: AsRef<Path>,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
```
$ sentry-cli mobile-app upload --help
? success
[EXPERIMENTAL] Upload mobile app files to a project.

Usage: sentry-cli[EXE] mobile-app upload [OPTIONS] <PATH>...

Arguments:
<PATH>... The path to the mobile app files to upload. Supported files include Apk, Aab,
XCArchive, and IPA.

Options:
-o, --org <ORG>
The organization ID or slug.
--header <KEY:VALUE>
Custom headers that should be attached to all requests
in key:value format.
-p, --project <PROJECT>
The project ID or slug.
--auth-token <AUTH_TOKEN>
Use the given Sentry auth token.
--sha <sha>
The git commit sha to use for the upload. If not provided, the current commit sha will be
used.
--build-configuration <build_configuration>
The build configuration to use for the upload. If not provided, the current version will
be used.
--log-level <LOG_LEVEL>
Set the log output verbosity. [possible values: trace, debug, info, warn, error]
--quiet
Do not print any output while preserving correct exit code. This flag is currently
implemented only for selected subcommands. [aliases: silent]
-h, --help
Print help

```
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ $ sentry-cli mobile-app upload --help
Usage: sentry-cli[EXE] mobile-app upload [OPTIONS] <PATH>...

Arguments:
<PATH>... The path to the mobile app files to upload. Supported files include Apk, Aab or
<PATH>... The path to the mobile app files to upload. Supported files include Apk, Aab, and
XCArchive.

Options:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```
$ sentry-cli mobile-app upload tests/integration/_fixtures/mobile_app/ipa.ipa --sha test_sha
? success
Successfully uploaded 1 file to Sentry
- tests/integration/_fixtures/mobile_app/ipa.ipa

```
1 change: 1 addition & 0 deletions tests/integration/_fixtures/mobile_app/invalid.ipa
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
invalid ipa content
Binary file added tests/integration/_fixtures/mobile_app/ipa.ipa
Binary file not shown.
Loading