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
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
threading the same four parameters through `move_destination_to_backup` (7 params, down to 4) and
`describe_final_swap_failure` (7 params, down to 4) to reach each of the six
`disclose_if_orphaned` call sites individually. No behavior change.
- **Sanitized file permission modes are now a distinct type, `SanitizedMode` (#549)**:
`security::sanitize_permissions` returns `SanitizedMode` instead of a plain `u32`, and
`ValidatedEntry::mode()`, `EntryValidator::validate_entry()`'s sanitized output, and
`formats::common::create_file_with_mode`/`extract_file_with_permit` now take `Option<SanitizedMode>`
instead of `Option<u32>`. `SanitizedMode` can only be constructed by `sanitize_permissions`, so an
unsanitized mode read from an archive header can no longer reach permission-setting code by mistake —
the invariant is enforced at compile time instead of only in a doc comment. Call `.as_u32()` to
recover the raw mode.
- **Six public enums expected to grow variants before v1.0.0 are now `#[non_exhaustive]` (#551)**:
`ArchiveError`, `QuotaResource`, `formats::detect::ArchiveType`, `formats::compression::CompressionCodec`,
`inspection::report::IssueCategory`, and `types::entry_type::EntryType`. (`creation::walker::EntryType`
is `pub(crate)`-only and not part of this change — the attribute would have no effect on a type that is
never nameable outside this crate.) Downstream crates matching on the six public enums exhaustively now
need a wildcard arm; `exarch-cli`, `exarch-python`, and `exarch-node` have been updated accordingly.

- **Bumped `sevenz-rust2` from 0.21.4 to 0.21.5, pulling in a transitive `lzma-rust2` bump from
0.18.0 to 0.19.0 (#548)**: `sevenz-rust2` 0.21.5 batches AES-CBC block decryption, a 7z-extraction
Expand Down
4 changes: 4 additions & 0 deletions crates/exarch-cli/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ pub fn convert_extraction_error(
archive.display(),
)
}
// Forward-compat: a variant added to ArchiveError after this match was
// written. #[non_exhaustive] requires this arm to compile against a
// newer exarch-core; there is no more specific context to add.
_ => format!("Error while processing '{}'", archive.display()),
};
anyhow::Error::from(err).context(context)
}
Expand Down
5 changes: 5 additions & 0 deletions crates/exarch-cli/src/output/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ fn extraction_error_kind(err: &ArchiveError) -> String {
ArchiveError::UnknownFormat { .. } => "UnknownFormat",
ArchiveError::InvalidConfiguration { .. } => "InvalidConfiguration",
ArchiveError::PartialExtraction { source, .. } => return extraction_error_kind(source),
// Forward-compat: a variant added to ArchiveError after this match was
// written. #[non_exhaustive] requires this arm to compile against a
// newer exarch-core. "Error" matches the generic fallback documented
// for kinds that don't map to a known archive validation failure.
_ => "Error",
}
.to_string()
}
Expand Down
8 changes: 8 additions & 0 deletions crates/exarch-core/src/error/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ use thiserror::Error;
pub type Result<T> = std::result::Result<T, ArchiveError>;

/// Represents a specific quota resource that was exceeded.
///
/// `#[non_exhaustive]` so a future quota dimension is not a breaking change
/// for downstream matches.
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
pub enum QuotaResource {
/// File count quota exceeded.
FileCount {
Expand Down Expand Up @@ -55,7 +59,11 @@ impl std::fmt::Display for QuotaResource {

/// Errors that can occur during archive operations (extraction, creation,
/// listing, verification).
///
/// `#[non_exhaustive]` so a future error variant is not a breaking change
/// for downstream matches.
#[derive(Error, Debug)]
#[non_exhaustive]
pub enum ArchiveError {
/// I/O operation failed.
#[error("I/O error: {0}")]
Expand Down
57 changes: 35 additions & 22 deletions crates/exarch-core/src/formats/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use crate::config::Validated;
use crate::copy::CopyBuffer;
use crate::copy::copy_with_buffer;
use crate::error::QuotaResource;
use crate::security::permissions::SanitizedMode;
use crate::security::quota::QuotaPermit;
use crate::types::DestDir;
use crate::types::SafePath;
Expand Down Expand Up @@ -542,14 +543,16 @@ pub fn check_extension_allowed(
/// - Strip sticky bit (0o1000) if required by security policy
/// - Ensure world-writable permissions are only set if allowed
///
/// Mode sanitization MUST be performed by the caller (typically in the
/// validation layer via `SecurityConfig::sanitize_mode()`). This function
/// does NOT perform any sanitization and will apply the mode value directly.
/// The [`SanitizedMode`] parameter type enforces mode sanitization at
/// compile time: only
/// [`sanitize_permissions`](crate::security::sanitize_permissions)
/// can construct one, so a raw, unsanitized mode read from an archive header
/// cannot reach this function by mistake.
///
/// # Arguments
///
/// * `path` - Path where file should be created
/// * `mode` - Optional Unix file mode (must be pre-sanitized by caller)
/// * `mode` - Optional pre-sanitized Unix file mode
/// * `create_new` - If `true`, fail with `AlreadyExists` instead of truncating
/// an existing file at `path`
///
Expand All @@ -563,7 +566,7 @@ pub fn check_extension_allowed(
#[cfg(unix)]
pub fn create_file_with_mode(
path: &Path,
mode: Option<u32>,
mode: Option<SanitizedMode>,
create_new: bool,
) -> std::io::Result<File> {
use std::fs::OpenOptions;
Expand All @@ -585,7 +588,7 @@ pub fn create_file_with_mode(

if let Some(m) = mode {
// Apply sanitized mode during open (already stripped setuid/setgid)
opts.mode(m);
opts.mode(m.as_u32());
}

let file = opts.open(path)?;
Expand All @@ -598,7 +601,7 @@ pub fn create_file_with_mode(
// TOCTOU window between this open() and the permission change (issue
// #460).
if let Some(m) = mode {
file.set_permissions(Permissions::from_mode(m))?;
file.set_permissions(Permissions::from_mode(m.as_u32()))?;
}

Ok(file)
Expand All @@ -625,7 +628,7 @@ pub fn create_file_with_mode(
#[cfg(not(unix))]
pub fn create_file_with_mode(
path: &Path,
_mode: Option<u32>,
_mode: Option<SanitizedMode>,
create_new: bool,
) -> std::io::Result<File> {
if create_new {
Expand Down Expand Up @@ -720,7 +723,7 @@ pub fn create_file_with_mode(
pub fn extract_file_with_permit<R: Read>(
reader: &mut R,
safe_path: &SafePath,
mode: Option<u32>,
mode: Option<SanitizedMode>,
_permit: QuotaPermit,
dest: &DestDir,
report: &mut ExtractionReport,
Expand Down Expand Up @@ -1079,12 +1082,22 @@ mod tests {
use crate::NoopProgress;
use crate::SecurityConfig;
use crate::copy::CopyBuffer;
use crate::security::permissions::sanitize_permissions;
use crate::security::quota::QuotaTracker;
use std::assert_matches;
use std::io::Cursor;
use std::path::PathBuf;
use tempfile::TempDir;

/// Builds a [`SanitizedMode`] for tests that don't otherwise need a
/// `SecurityConfig` in scope. None of the modes used across these tests
/// carry setuid/setgid/world-writable bits, so sanitizing with the
/// default config never changes the value.
fn sanitized(mode: u32) -> SanitizedMode {
let config = SecurityConfig::default().validate().expect("valid config");
sanitize_permissions(mode, &config)
}

#[test]
fn test_extract_file_with_permit_integer_overflow_check() {
let temp = TempDir::new().expect("failed to create temp dir");
Expand All @@ -1111,7 +1124,7 @@ mod tests {
let result = extract_file_with_permit(
&mut reader,
&safe_path,
Some(0o644),
Some(sanitized(0o644)),
permit,
&dest,
&mut report,
Expand Down Expand Up @@ -1175,7 +1188,7 @@ mod tests {
let result = extract_file_with_permit(
&mut reader,
&safe_path,
Some(0o644),
Some(sanitized(0o644)),
permit,
&dest,
&mut report,
Expand Down Expand Up @@ -1224,7 +1237,7 @@ mod tests {
let result = extract_file_with_permit(
&mut reader,
&safe_path,
Some(0o644),
Some(sanitized(0o644)),
permit,
&dest,
&mut report,
Expand Down Expand Up @@ -1270,7 +1283,7 @@ mod tests {
let result = extract_file_with_permit(
&mut reader,
&safe_path,
Some(0o644),
Some(sanitized(0o644)),
permit,
&dest,
&mut report,
Expand Down Expand Up @@ -1327,7 +1340,7 @@ mod tests {
let result = extract_file_with_permit(
&mut reader,
&safe_path,
Some(0o644),
Some(sanitized(0o644)),
permit,
&dest,
&mut report,
Expand Down Expand Up @@ -1557,8 +1570,8 @@ mod tests {
let file_path = temp.path().join("test_0o644.txt");

// Create file with mode 0o644
let file =
create_file_with_mode(&file_path, Some(0o644), false).expect("should create file");
let file = create_file_with_mode(&file_path, Some(sanitized(0o644)), false)
.expect("should create file");
drop(file);

// Verify file exists
Expand Down Expand Up @@ -1586,8 +1599,8 @@ mod tests {
let file_path = temp.path().join("test_0o755.txt");

// Create file with mode 0o755
let file =
create_file_with_mode(&file_path, Some(0o755), false).expect("should create file");
let file = create_file_with_mode(&file_path, Some(sanitized(0o755)), false)
.expect("should create file");
drop(file);

// Verify file exists
Expand Down Expand Up @@ -1615,8 +1628,8 @@ mod tests {
let file_path = temp.path().join("test_0o600.txt");

// Create file with mode 0o600
let file =
create_file_with_mode(&file_path, Some(0o600), false).expect("should create file");
let file = create_file_with_mode(&file_path, Some(sanitized(0o600)), false)
.expect("should create file");
drop(file);

// Verify file exists
Expand Down Expand Up @@ -1703,7 +1716,7 @@ mod tests {
let config = SecurityConfig::default().validate().expect("valid config");

// Mode 0o777 in archive, sanitized to 0o775 (world-writable stripped)
let sanitized_mode = 0o775u32;
let sanitized_mode = sanitize_permissions(0o777, &config);
let permit = QuotaTracker::new()
.reserve(0, &config)
.expect("reservation should succeed");
Expand Down Expand Up @@ -1763,7 +1776,7 @@ mod tests {
// process-global but safe to mutate here. Restored unconditionally.
let previous_umask = unsafe { libc::umask(0o077) };

let result = create_file_with_mode(&file_path, Some(0o755), false);
let result = create_file_with_mode(&file_path, Some(sanitized(0o755)), false);

// Restore previous umask unconditionally before any assert.
unsafe { libc::umask(previous_umask) };
Expand Down
4 changes: 4 additions & 0 deletions crates/exarch-core/src/formats/compression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@
/// let best_codec = CompressionCodec::Xz; // Best compression ratio
/// let modern_codec = CompressionCodec::Zstd; // Modern balanced approach
/// ```
///
/// `#[non_exhaustive]` so support for a new compression codec is not a
/// breaking change for downstream matches.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[non_exhaustive]
pub enum CompressionCodec {
/// Gzip compression (deflate algorithm).
///
Expand Down
4 changes: 4 additions & 0 deletions crates/exarch-core/src/formats/detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ pub(crate) fn is_zip_family_alias(ext: &str) -> bool {
}

/// Supported archive formats.
///
/// `#[non_exhaustive]` so support for a new archive format is not a
/// breaking change for downstream matches.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[non_exhaustive]
pub enum ArchiveType {
/// Tar archive (uncompressed).
Tar,
Expand Down
3 changes: 2 additions & 1 deletion crates/exarch-core/src/formats/tar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ use crate::Result;
use crate::SecurityConfig;
use crate::config::Validated;
use crate::copy::CopyBuffer;
use crate::security::permissions::SanitizedMode;
use crate::security::quota::QuotaPermit;
use crate::security::validator::EntryValidator;
use crate::security::validator::ValidatedEntryType;
Expand Down Expand Up @@ -288,7 +289,7 @@ impl<R: Read> TarArchive<R> {
fn extract_file<ER: Read>(
entry: &mut tar::Entry<'_, ER>,
safe_path: &SafePath,
mode: Option<u32>,
mode: Option<SanitizedMode>,
permit: QuotaPermit,
ctx: &mut ExtractionContext<'_, '_>,
) -> Result<()> {
Expand Down
3 changes: 2 additions & 1 deletion crates/exarch-core/src/formats/zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ use crate::SecurityConfig;
use crate::config::Validated;
use crate::copy::CopyBuffer;
use crate::security::EntryValidator;
use crate::security::permissions::SanitizedMode;
use crate::security::quota::QuotaPermit;
use crate::security::validator::ValidatedEntryType;
use crate::types::DestDir;
Expand Down Expand Up @@ -453,7 +454,7 @@ impl<R: Read + Seek> ZipArchive<R> {
fn extract_file(
zip_file: &mut zip::read::ZipFile<'_, R>,
safe_path: &SafePath,
mode: Option<u32>,
mode: Option<SanitizedMode>,
permit: QuotaPermit,
file_size: u64,
ctx: &mut ZipExtractionContext<'_>,
Expand Down
4 changes: 4 additions & 0 deletions crates/exarch-core/src/inspection/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,11 @@ impl std::fmt::Display for IssueSeverity {
}

/// Issue categories (maps to security checks).
///
/// `#[non_exhaustive]` so a future security check is not a breaking change
/// for downstream matches.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[non_exhaustive]
pub enum IssueCategory {
/// Path traversal attack
PathTraversal,
Expand Down
2 changes: 1 addition & 1 deletion crates/exarch-core/src/inspection/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ fn verify_entry(

fn check_permissions(path: &Path, mode: u32, config: &SecurityConfig<Validated>) -> Result<()> {
let sanitized = sanitize_permissions(mode, config);
if sanitized == mode {
if sanitized.as_u32() == mode {
Ok(())
} else {
Err(ArchiveError::InvalidPermissions {
Expand Down
5 changes: 5 additions & 0 deletions crates/exarch-core/src/security/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ pub use validator::ValidationReport;
// would be a private-type-in-public-interface compile error.
pub use quota::QuotaPermit;

// SanitizedMode rides inside the unconditionally-public
// ValidatedEntry::mode(), so — like QuotaPermit above — it must be exported
// ungated rather than gated behind `testing`.
pub use permissions::SanitizedMode;

// Security primitives exposed under the `testing` feature for external
// benchmarks and integration tests that cannot access pub(crate) items.
#[cfg(feature = "testing")]
Expand Down
Loading