From 7d47056ea42636271ac020b86347fbbef49490aa Mon Sep 17 00:00:00 2001 From: xl-openai Date: Fri, 22 May 2026 19:31:39 -0700 Subject: [PATCH] fix: plugin bundle archive handling for upload and install (#23983) Move plugin tar.gz packing and unpacking into a shared core-plugins archive helper so uploaded bundles are decoded through the same tar handling used for installs. This removes duplicate archive logic, supports GNU long-name entries on extraction, and keeps size, traversal, link, and entry-type checks in one place. --- codex-rs/core-plugins/src/lib.rs | 1 + .../core-plugins/src/plugin_bundle_archive.rs | 315 ++++++++++++++++++ codex-rs/core-plugins/src/remote/share.rs | 149 +-------- .../core-plugins/src/remote/share/tests.rs | 28 ++ codex-rs/core-plugins/src/remote_bundle.rs | 209 +++--------- 5 files changed, 403 insertions(+), 299 deletions(-) create mode 100644 codex-rs/core-plugins/src/plugin_bundle_archive.rs diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index b724699039..4e59162e34 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -6,6 +6,7 @@ pub mod marketplace; pub mod marketplace_add; pub mod marketplace_remove; pub mod marketplace_upgrade; +mod plugin_bundle_archive; pub mod remote; pub mod remote_bundle; pub mod remote_legacy; diff --git a/codex-rs/core-plugins/src/plugin_bundle_archive.rs b/codex-rs/core-plugins/src/plugin_bundle_archive.rs new file mode 100644 index 0000000000..b29117d102 --- /dev/null +++ b/codex-rs/core-plugins/src/plugin_bundle_archive.rs @@ -0,0 +1,315 @@ +use flate2::Compression; +use flate2::read::GzDecoder; +use flate2::write::GzEncoder; +use std::fmt; +use std::fs; +use std::io; +use std::io::Read; +use std::io::Write; +use std::path::Path; +use std::path::PathBuf; +use tar::Archive; + +#[derive(Debug, thiserror::Error)] +pub(crate) enum PluginBundlePackError { + #[error("invalid plugin path `{path}`: {reason}")] + InvalidPluginPath { path: PathBuf, reason: String }, + + #[error("plugin archive would be {bytes} bytes, exceeding maximum size of {max_bytes} bytes")] + ArchiveTooLarge { bytes: usize, max_bytes: usize }, + + #[error("failed to archive plugin bundle: {source}")] + Io { + #[source] + source: io::Error, + }, +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum PluginBundleUnpackError { + #[error( + "plugin bundle extracted size would be {bytes} bytes, exceeding maximum total size of {max_bytes} bytes" + )] + ExtractedBundleTooLarge { bytes: u64, max_bytes: u64 }, + + #[error("{context}: {source}")] + Io { + context: &'static str, + #[source] + source: io::Error, + }, + + #[error("{0}")] + InvalidBundle(String), +} + +impl PluginBundleUnpackError { + fn io(context: &'static str, source: io::Error) -> Self { + Self::Io { context, source } + } +} + +pub(crate) fn pack_plugin_bundle_tar_gz( + plugin_path: &Path, + max_bytes: usize, +) -> Result, PluginBundlePackError> { + if !plugin_path.is_dir() { + return Err(PluginBundlePackError::InvalidPluginPath { + path: plugin_path.to_path_buf(), + reason: "expected a plugin directory".to_string(), + }); + } + if !plugin_path.join(".codex-plugin/plugin.json").is_file() { + return Err(PluginBundlePackError::InvalidPluginPath { + path: plugin_path.to_path_buf(), + reason: "missing .codex-plugin/plugin.json".to_string(), + }); + } + + let encoder = GzEncoder::new(SizeLimitedBuffer::new(max_bytes), Compression::default()); + let mut archive = tar::Builder::new(encoder); + append_plugin_tree(&mut archive, plugin_path, plugin_path).map_err(archive_io_error)?; + let encoder = archive.into_inner().map_err(archive_io_error)?; + encoder + .finish() + .map(SizeLimitedBuffer::into_inner) + .map_err(archive_io_error) +} + +fn append_plugin_tree( + archive: &mut tar::Builder, + plugin_root: &Path, + current: &Path, +) -> io::Result<()> { + let mut entries = fs::read_dir(current)?.collect::, io::Error>>()?; + entries.sort_by_key(fs::DirEntry::file_name); + for entry in entries { + let path = entry.path(); + let file_type = entry.file_type()?; + let relative_path = path.strip_prefix(plugin_root).map_err(|err| { + io::Error::other(format!( + "failed to compute plugin archive path for `{}`: {err}", + path.display() + )) + })?; + if file_type.is_dir() { + archive.append_dir(relative_path, &path)?; + append_plugin_tree(archive, plugin_root, &path)?; + } else if file_type.is_file() { + archive.append_path_with_name(&path, relative_path)?; + } else { + return Err(io::Error::other(format!( + "unsupported plugin archive entry type: {}", + path.display() + ))); + } + } + Ok(()) +} + +fn archive_io_error(source: io::Error) -> PluginBundlePackError { + if let Some(limit) = source + .get_ref() + .and_then(|err| err.downcast_ref::()) + { + return PluginBundlePackError::ArchiveTooLarge { + bytes: limit.bytes, + max_bytes: limit.max_bytes, + }; + } + + PluginBundlePackError::Io { source } +} + +pub(crate) fn unpack_plugin_bundle_tar_gz( + bytes: &[u8], + destination: &Path, + max_total_bytes: u64, +) -> Result<(), PluginBundleUnpackError> { + fs::create_dir_all(destination).map_err(|source| { + PluginBundleUnpackError::io( + "failed to create plugin bundle extraction directory", + source, + ) + })?; + + let archive = GzDecoder::new(std::io::Cursor::new(bytes)); + let mut archive = Archive::new(archive); + unpack_plugin_bundle_tar(&mut archive, destination, max_total_bytes) +} + +fn unpack_plugin_bundle_tar( + archive: &mut Archive, + destination: &Path, + max_total_bytes: u64, +) -> Result<(), PluginBundleUnpackError> { + let mut extracted_bytes = 0u64; + let entries = archive.entries().map_err(|source| { + PluginBundleUnpackError::io("failed to read plugin bundle tar", source) + })?; + for entry in entries { + let mut entry = entry.map_err(|source| { + PluginBundleUnpackError::io("failed to read plugin bundle tar entry", source) + })?; + let entry_type = entry.header().entry_type(); + let entry_size = entry.size(); + let entry_path = entry + .path() + .map_err(|source| { + PluginBundleUnpackError::io("failed to read plugin bundle tar entry path", source) + })? + .into_owned(); + let output_path = checked_tar_output_path(destination, &entry_path)?; + + if entry_type.is_dir() { + fs::create_dir_all(&output_path).map_err(|source| { + PluginBundleUnpackError::io("failed to create plugin bundle directory", source) + })?; + continue; + } + + if entry_type.is_file() { + enforce_total_extracted_size(entry_size, &mut extracted_bytes, max_total_bytes)?; + let Some(parent) = output_path.parent() else { + return Err(PluginBundleUnpackError::InvalidBundle(format!( + "plugin bundle output path has no parent: {}", + output_path.display() + ))); + }; + fs::create_dir_all(parent).map_err(|source| { + PluginBundleUnpackError::io("failed to create plugin bundle directory", source) + })?; + entry.unpack(&output_path).map_err(|source| { + PluginBundleUnpackError::io("failed to unpack plugin bundle entry", source) + })?; + continue; + } + + if entry_type.is_hard_link() || entry_type.is_symlink() { + return Err(PluginBundleUnpackError::InvalidBundle(format!( + "plugin bundle tar entry `{}` is a link", + entry_path.display() + ))); + } + + return Err(PluginBundleUnpackError::InvalidBundle(format!( + "plugin bundle tar entry `{}` has unsupported type {:?}", + entry_path.display(), + entry_type + ))); + } + + Ok(()) +} + +fn checked_tar_output_path( + destination: &Path, + entry_name: &Path, +) -> Result { + let mut output_path = destination.to_path_buf(); + let mut has_component = false; + for component in entry_name.components() { + match component { + std::path::Component::Normal(component) => { + has_component = true; + output_path.push(component); + } + std::path::Component::CurDir => {} + std::path::Component::ParentDir + | std::path::Component::RootDir + | std::path::Component::Prefix(_) => { + return Err(PluginBundleUnpackError::InvalidBundle(format!( + "plugin bundle tar entry `{}` escapes extraction root", + entry_name.display() + ))); + } + } + } + if !has_component { + return Err(PluginBundleUnpackError::InvalidBundle( + "plugin bundle tar entry has an empty path".to_string(), + )); + } + Ok(output_path) +} + +fn enforce_total_extracted_size( + entry_size: u64, + extracted_bytes: &mut u64, + max_total_bytes: u64, +) -> Result<(), PluginBundleUnpackError> { + let next_total = extracted_bytes.checked_add(entry_size).ok_or( + PluginBundleUnpackError::ExtractedBundleTooLarge { + bytes: u64::MAX, + max_bytes: max_total_bytes, + }, + )?; + if next_total > max_total_bytes { + return Err(PluginBundleUnpackError::ExtractedBundleTooLarge { + bytes: next_total, + max_bytes: max_total_bytes, + }); + } + *extracted_bytes = next_total; + Ok(()) +} + +struct SizeLimitedBuffer { + bytes: Vec, + max_bytes: usize, +} + +impl SizeLimitedBuffer { + fn new(max_bytes: usize) -> Self { + Self { + bytes: Vec::new(), + max_bytes, + } + } + + fn into_inner(self) -> Vec { + self.bytes + } +} + +impl Write for SizeLimitedBuffer { + fn write(&mut self, buf: &[u8]) -> io::Result { + let next_len = self.bytes.len().checked_add(buf.len()).ok_or_else(|| { + io::Error::other(ArchiveSizeLimitExceeded { + bytes: usize::MAX, + max_bytes: self.max_bytes, + }) + })?; + if next_len > self.max_bytes { + return Err(io::Error::other(ArchiveSizeLimitExceeded { + bytes: next_len, + max_bytes: self.max_bytes, + })); + } + + self.bytes.extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } +} + +#[derive(Debug)] +struct ArchiveSizeLimitExceeded { + bytes: usize, + max_bytes: usize, +} + +impl fmt::Display for ArchiveSizeLimitExceeded { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "archive would be {} bytes, exceeding maximum size of {} bytes", + self.bytes, self.max_bytes + ) + } +} + +impl std::error::Error for ArchiveSizeLimitExceeded {} diff --git a/codex-rs/core-plugins/src/remote/share.rs b/codex-rs/core-plugins/src/remote/share.rs index 9b5a2572a4..c400d27c47 100644 --- a/codex-rs/core-plugins/src/remote/share.rs +++ b/codex-rs/core-plugins/src/remote/share.rs @@ -1,18 +1,15 @@ use super::*; +use crate::plugin_bundle_archive::PluginBundlePackError; +use crate::plugin_bundle_archive::pack_plugin_bundle_tar_gz; use codex_login::CodexAuth; use codex_login::default_client::build_reqwest_client; use codex_utils_absolute_path::AbsolutePathBuf; -use flate2::Compression; -use flate2::write::GzEncoder; use reqwest::RequestBuilder; use reqwest::StatusCode; use serde::Deserialize; use serde::Serialize; use std::collections::BTreeMap; -use std::fmt; -use std::fs; use std::io; -use std::io::Write; use std::path::Path; use tracing::warn; @@ -477,140 +474,20 @@ fn archive_plugin_for_upload_with_limit( plugin_path: &Path, max_bytes: usize, ) -> Result, RemotePluginCatalogError> { - if !plugin_path.is_dir() { - return Err(RemotePluginCatalogError::InvalidPluginPath { + pack_plugin_bundle_tar_gz(plugin_path, max_bytes).map_err(|err| match err { + PluginBundlePackError::InvalidPluginPath { path, reason } => { + RemotePluginCatalogError::InvalidPluginPath { path, reason } + } + PluginBundlePackError::ArchiveTooLarge { bytes, max_bytes } => { + RemotePluginCatalogError::ArchiveTooLarge { bytes, max_bytes } + } + PluginBundlePackError::Io { source } => RemotePluginCatalogError::Archive { path: plugin_path.to_path_buf(), - reason: "expected a plugin directory".to_string(), - }); - } - if !plugin_path.join(".codex-plugin/plugin.json").is_file() { - return Err(RemotePluginCatalogError::InvalidPluginPath { - path: plugin_path.to_path_buf(), - reason: "missing .codex-plugin/plugin.json".to_string(), - }); - } - - let encoder = GzEncoder::new(SizeLimitedBuffer::new(max_bytes), Compression::default()); - let mut archive = tar::Builder::new(encoder); - append_plugin_tree(&mut archive, plugin_path, plugin_path) - .map_err(|source| archive_error(plugin_path, source))?; - let encoder = archive - .into_inner() - .map_err(|source| archive_error(plugin_path, source))?; - encoder - .finish() - .map(SizeLimitedBuffer::into_inner) - .map_err(|source| archive_error(plugin_path, source)) + source, + }, + }) } -fn append_plugin_tree( - archive: &mut tar::Builder, - plugin_root: &Path, - current: &Path, -) -> io::Result<()> { - let mut entries = fs::read_dir(current)?.collect::, io::Error>>()?; - entries.sort_by_key(fs::DirEntry::file_name); - for entry in entries { - let path = entry.path(); - let file_type = entry.file_type()?; - let relative_path = path.strip_prefix(plugin_root).map_err(|err| { - io::Error::other(format!( - "failed to compute plugin archive path for `{}`: {err}", - path.display() - )) - })?; - if file_type.is_dir() { - archive.append_dir(relative_path, &path)?; - append_plugin_tree(archive, plugin_root, &path)?; - } else if file_type.is_file() { - archive.append_path_with_name(&path, relative_path)?; - } else { - return Err(io::Error::other(format!( - "unsupported plugin archive entry type: {}", - path.display() - ))); - } - } - Ok(()) -} - -fn archive_error(plugin_path: &Path, source: io::Error) -> RemotePluginCatalogError { - if let Some(limit) = source - .get_ref() - .and_then(|err| err.downcast_ref::()) - { - return RemotePluginCatalogError::ArchiveTooLarge { - bytes: limit.bytes, - max_bytes: limit.max_bytes, - }; - } - - RemotePluginCatalogError::Archive { - path: plugin_path.to_path_buf(), - source, - } -} - -struct SizeLimitedBuffer { - bytes: Vec, - max_bytes: usize, -} - -impl SizeLimitedBuffer { - fn new(max_bytes: usize) -> Self { - Self { - bytes: Vec::new(), - max_bytes, - } - } - - fn into_inner(self) -> Vec { - self.bytes - } -} - -impl Write for SizeLimitedBuffer { - fn write(&mut self, buf: &[u8]) -> io::Result { - let next_len = self.bytes.len().checked_add(buf.len()).ok_or_else(|| { - io::Error::other(ArchiveSizeLimitExceeded { - bytes: usize::MAX, - max_bytes: self.max_bytes, - }) - })?; - if next_len > self.max_bytes { - return Err(io::Error::other(ArchiveSizeLimitExceeded { - bytes: next_len, - max_bytes: self.max_bytes, - })); - } - - self.bytes.extend_from_slice(buf); - Ok(buf.len()) - } - - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } -} - -#[derive(Debug)] -struct ArchiveSizeLimitExceeded { - bytes: usize, - max_bytes: usize, -} - -impl fmt::Display for ArchiveSizeLimitExceeded { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "archive would be {} bytes, exceeding maximum size of {} bytes", - self.bytes, self.max_bytes - ) - } -} - -impl std::error::Error for ArchiveSizeLimitExceeded {} - async fn send_and_expect_status( request: RequestBuilder, url_for_error: &str, diff --git a/codex-rs/core-plugins/src/remote/share/tests.rs b/codex-rs/core-plugins/src/remote/share/tests.rs index f62051374e..e33d3c5d51 100644 --- a/codex-rs/core-plugins/src/remote/share/tests.rs +++ b/codex-rs/core-plugins/src/remote/share/tests.rs @@ -326,6 +326,34 @@ fn archive_plugin_for_upload_places_manifest_at_archive_root() { ); } +#[test] +fn archive_plugin_for_upload_round_trips_through_plugin_bundle_archive_with_long_paths() { + let temp_dir = TempDir::new().unwrap(); + let plugin_path = write_test_plugin(temp_dir.path(), "demo-plugin"); + let long_skill_path = Path::new("skills") + .join(["segment"; 40].join("/")) + .join("SKILL.md"); + write_file(&plugin_path.join(&long_skill_path), "# Long path skill\n"); + + let archive_bytes = archive_plugin_for_upload(&plugin_path).unwrap(); + let destination = TempDir::new().unwrap(); + crate::plugin_bundle_archive::unpack_plugin_bundle_tar_gz( + &archive_bytes, + destination.path(), + /*max_total_bytes*/ 1024 * 1024, + ) + .expect("extract shared plugin archive"); + + assert_eq!( + fs::read_to_string(destination.path().join(".codex-plugin/plugin.json")).unwrap(), + r#"{"name":"demo-plugin"}"# + ); + assert_eq!( + fs::read_to_string(destination.path().join(long_skill_path)).unwrap(), + "# Long path skill\n" + ); +} + #[tokio::test] async fn save_remote_plugin_share_updates_existing_workspace_plugin() { let codex_home = TempDir::new().unwrap(); diff --git a/codex-rs/core-plugins/src/remote_bundle.rs b/codex-rs/core-plugins/src/remote_bundle.rs index 7862573e44..cb60d60b77 100644 --- a/codex-rs/core-plugins/src/remote_bundle.rs +++ b/codex-rs/core-plugins/src/remote_bundle.rs @@ -1,3 +1,5 @@ +use crate::plugin_bundle_archive::PluginBundleUnpackError; +use crate::plugin_bundle_archive::unpack_plugin_bundle_tar_gz; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::store::PluginInstallResult; use crate::store::PluginStore; @@ -8,17 +10,14 @@ use codex_plugin::PluginId; use codex_plugin::PluginIdError; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::find_plugin_manifest_path; -use flate2::read::GzDecoder; use reqwest::Response; use reqwest::StatusCode; use serde_json::Value as JsonValue; use std::fs; use std::io; -use std::io::Read; use std::path::Path; use std::path::PathBuf; use std::time::Duration; -use tar::Archive; use url::Host; use url::Url; @@ -542,146 +541,17 @@ fn extract_plugin_bundle_tar_gz_with_limits( destination: &Path, max_total_bytes: u64, ) -> Result<(), RemotePluginBundleInstallError> { - fs::create_dir_all(destination).map_err(|source| { - RemotePluginBundleInstallError::io( - "failed to create remote plugin bundle extraction directory", - source, - ) - })?; - - let archive = GzDecoder::new(std::io::Cursor::new(bytes)); - let mut archive = Archive::new(archive); - extract_plugin_bundle_tar(&mut archive, destination, max_total_bytes) -} - -fn extract_plugin_bundle_tar( - archive: &mut Archive, - destination: &Path, - max_total_bytes: u64, -) -> Result<(), RemotePluginBundleInstallError> { - let mut extracted_bytes = 0u64; - let entries = archive.entries().map_err(|source| { - RemotePluginBundleInstallError::io("failed to read remote plugin bundle tar", source) - })?; - let entries = entries.raw(true); - for entry in entries { - let mut entry = entry.map_err(|source| { - RemotePluginBundleInstallError::io( - "failed to read remote plugin bundle tar entry", - source, - ) - })?; - let entry_type = entry.header().entry_type(); - let entry_size = entry.size(); - let entry_path = entry.path().map_err(|source| { - RemotePluginBundleInstallError::io( - "failed to read remote plugin bundle tar entry path", - source, - ) - })?; - let entry_path = entry_path.into_owned(); - let output_path = checked_tar_output_path(destination, &entry_path)?; - - if entry_type.is_dir() { - fs::create_dir_all(&output_path).map_err(|source| { - RemotePluginBundleInstallError::io( - "failed to create remote plugin bundle directory", - source, - ) - })?; - continue; + unpack_plugin_bundle_tar_gz(bytes, destination, max_total_bytes).map_err(|err| match err { + PluginBundleUnpackError::ExtractedBundleTooLarge { bytes, max_bytes } => { + RemotePluginBundleInstallError::ExtractedBundleTooLarge { bytes, max_bytes } } - - if entry_type.is_file() { - enforce_total_extracted_size(entry_size, &mut extracted_bytes, max_total_bytes)?; - let Some(parent) = output_path.parent() else { - return Err(RemotePluginBundleInstallError::InvalidBundle(format!( - "remote plugin bundle output path has no parent: {}", - output_path.display() - ))); - }; - fs::create_dir_all(parent).map_err(|source| { - RemotePluginBundleInstallError::io( - "failed to create remote plugin bundle directory", - source, - ) - })?; - entry.unpack(&output_path).map_err(|source| { - RemotePluginBundleInstallError::io( - "failed to unpack remote plugin bundle entry", - source, - ) - })?; - continue; + PluginBundleUnpackError::Io { context, source } => { + RemotePluginBundleInstallError::io(context, source) } - - if entry_type.is_hard_link() || entry_type.is_symlink() { - return Err(RemotePluginBundleInstallError::InvalidBundle(format!( - "remote plugin bundle tar entry `{}` is a link", - entry_path.display() - ))); + PluginBundleUnpackError::InvalidBundle(message) => { + RemotePluginBundleInstallError::InvalidBundle(message) } - - return Err(RemotePluginBundleInstallError::InvalidBundle(format!( - "remote plugin bundle tar entry `{}` has unsupported type {:?}", - entry_path.display(), - entry_type - ))); - } - - Ok(()) -} - -fn checked_tar_output_path( - destination: &Path, - entry_name: &Path, -) -> Result { - let mut output_path = destination.to_path_buf(); - let mut has_component = false; - for component in entry_name.components() { - match component { - std::path::Component::Normal(component) => { - has_component = true; - output_path.push(component); - } - std::path::Component::CurDir => {} - std::path::Component::ParentDir - | std::path::Component::RootDir - | std::path::Component::Prefix(_) => { - return Err(RemotePluginBundleInstallError::InvalidBundle(format!( - "remote plugin bundle tar entry `{}` escapes extraction root", - entry_name.display() - ))); - } - } - } - if !has_component { - return Err(RemotePluginBundleInstallError::InvalidBundle( - "remote plugin bundle tar entry has an empty path".to_string(), - )); - } - Ok(output_path) -} - -fn enforce_total_extracted_size( - entry_size: u64, - extracted_bytes: &mut u64, - max_total_bytes: u64, -) -> Result<(), RemotePluginBundleInstallError> { - let next_total = extracted_bytes.checked_add(entry_size).ok_or( - RemotePluginBundleInstallError::ExtractedBundleTooLarge { - bytes: u64::MAX, - max_bytes: max_total_bytes, - }, - )?; - if next_total > max_total_bytes { - return Err(RemotePluginBundleInstallError::ExtractedBundleTooLarge { - bytes: next_total, - max_bytes: max_total_bytes, - }); - } - *extracted_bytes = next_total; - Ok(()) + }) } fn find_extracted_plugin_root( @@ -706,6 +576,7 @@ mod tests { use flate2::Compression; use flate2::write::GzEncoder; use pretty_assertions::assert_eq; + use std::io::Write; use tempfile::tempdir; const REMOTE_PLUGIN_ID: &str = "plugins~Plugin_00000000000000000000000000000000"; @@ -830,7 +701,7 @@ mod tests { ) .expect_err("invalid tar.gz should be rejected"); - assert!(format!("{err}").contains("failed to read remote plugin bundle tar")); + assert!(format!("{err}").contains("failed to read plugin bundle tar")); } #[test] @@ -961,8 +832,11 @@ mod tests { #[test] fn extraction_rejects_tar_path_traversal() { let destination = tempdir().expect("tempdir"); - let err = checked_tar_output_path(destination.path(), Path::new("../evil.txt")) - .expect_err("tar path traversal should be rejected"); + let err = extract_plugin_bundle_tar_gz( + &tar_gz_bytes_with_raw_path("../evil.txt", b"evil", /*mode*/ 0o644), + destination.path(), + ) + .expect_err("tar path traversal should be rejected"); assert!(format!("{err}").contains("escapes extraction root")); } @@ -987,20 +861,20 @@ mod tests { } #[test] - fn extraction_rejects_pax_metadata_entries() { + fn extraction_supports_gnu_long_name_entries() { let destination = tempdir().expect("tempdir"); - let err = extract_plugin_bundle_tar_gz( - &tar_gz_bytes_with_entry_type( - tar::EntryType::XHeader, - "PaxHeaders.0/linear", - b"18 path=linear\n", - /*mode*/ 0o644, - ), + let long_path = format!("{}/file.txt", ["segment"; 40].join("/")); + + extract_plugin_bundle_tar_gz( + &tar_gz_bytes(&[(long_path.as_str(), b"long", /*mode*/ 0o644)]), destination.path(), ) - .expect_err("pax metadata entries should be rejected"); + .expect("extract bundle with GNU long name entry"); - assert!(format!("{err}").contains("unsupported type")); + assert_eq!( + std::fs::read(destination.path().join(long_path)).expect("read extracted file"), + b"long" + ); } #[cfg(unix)] @@ -1051,16 +925,25 @@ mod tests { finish_tar_gz(tar) } - fn tar_gz_bytes_with_entry_type( - entry_type: tar::EntryType, - path: &str, - contents: &[u8], - mode: u32, - ) -> Vec { - let encoder = GzEncoder::new(Vec::new(), Compression::default()); - let mut tar = tar::Builder::new(encoder); - append_tar_entry(&mut tar, entry_type, path, contents, mode); - finish_tar_gz(tar) + fn tar_gz_bytes_with_raw_path(path: &str, contents: &[u8], mode: u32) -> Vec { + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Regular); + header.set_size(contents.len() as u64); + header.set_mode(mode); + header.as_mut_bytes()[..path.len()].copy_from_slice(path.as_bytes()); + header.set_cksum(); + + let mut encoder = GzEncoder::new(Vec::new(), Compression::default()); + encoder + .write_all(header.as_bytes()) + .expect("write tar header"); + encoder.write_all(contents).expect("write tar contents"); + let padding = (512 - (contents.len() % 512)) % 512; + encoder + .write_all(&vec![0; padding]) + .expect("write tar padding"); + encoder.write_all(&[0; 1024]).expect("write tar terminator"); + encoder.finish().expect("finish gzip") } fn append_tar_entry(