From 53fc488affbdd78b13ecf9c43989236ac011fb88 Mon Sep 17 00:00:00 2001 From: "Andrei G." Date: Tue, 18 Aug 2026 20:29:30 +0200 Subject: [PATCH] feat(core): add SanitizedMode newtype and mark growth-prone enums non_exhaustive Wrap sanitized permission modes in a SanitizedMode newtype so a caller can no longer pass an unsanitized u32 into create_file_with_mode or extract_file_with_permit; the invariant was previously enforced only by a doc-comment contract. SanitizedMode can only be constructed by sanitize_permissions, matching the SafePath/QuotaPermit sealed-type pattern already used in this crate. Mark ArchiveError, QuotaResource, ArchiveType, CompressionCodec, IssueCategory, and types::entry_type::EntryType as #[non_exhaustive] so adding a variant to any of them is no longer a semver break for downstream exhaustive matches, consistent with ValidatedEntryType. Closes #549 Closes #551 --- CHANGELOG.md | 14 +++ crates/exarch-cli/src/error.rs | 4 + crates/exarch-cli/src/output/json.rs | 5 + crates/exarch-core/src/error/types.rs | 8 ++ crates/exarch-core/src/formats/common.rs | 57 ++++++----- crates/exarch-core/src/formats/compression.rs | 4 + crates/exarch-core/src/formats/detect.rs | 4 + crates/exarch-core/src/formats/tar.rs | 3 +- crates/exarch-core/src/formats/zip.rs | 3 +- crates/exarch-core/src/inspection/report.rs | 4 + crates/exarch-core/src/inspection/verify.rs | 2 +- crates/exarch-core/src/security/mod.rs | 5 + .../exarch-core/src/security/permissions.rs | 99 ++++++++++++++----- crates/exarch-core/src/security/validator.rs | 13 +-- crates/exarch-core/src/types/entry_type.rs | 4 + .../sanitized_mode_forge_via_tuple_struct.rs | 11 +++ ...nitized_mode_forge_via_tuple_struct.stderr | 11 +++ crates/exarch-node/src/error.rs | 11 +++ crates/exarch-python/src/error.rs | 8 ++ 19 files changed, 216 insertions(+), 54 deletions(-) create mode 100644 crates/exarch-core/tests/ui/sanitized_mode_forge_via_tuple_struct.rs create mode 100644 crates/exarch-core/tests/ui/sanitized_mode_forge_via_tuple_struct.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e62a1b..68d00d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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` + instead of `Option`. `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 diff --git a/crates/exarch-cli/src/error.rs b/crates/exarch-cli/src/error.rs index 311e3c1..a368a24 100644 --- a/crates/exarch-cli/src/error.rs +++ b/crates/exarch-cli/src/error.rs @@ -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) } diff --git a/crates/exarch-cli/src/output/json.rs b/crates/exarch-cli/src/output/json.rs index 91ac245..68305cf 100644 --- a/crates/exarch-cli/src/output/json.rs +++ b/crates/exarch-cli/src/output/json.rs @@ -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() } diff --git a/crates/exarch-core/src/error/types.rs b/crates/exarch-core/src/error/types.rs index 9595f15..fee6e1c 100644 --- a/crates/exarch-core/src/error/types.rs +++ b/crates/exarch-core/src/error/types.rs @@ -7,7 +7,11 @@ use thiserror::Error; pub type Result = std::result::Result; /// 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 { @@ -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}")] diff --git a/crates/exarch-core/src/formats/common.rs b/crates/exarch-core/src/formats/common.rs index c420c35..69c9198 100644 --- a/crates/exarch-core/src/formats/common.rs +++ b/crates/exarch-core/src/formats/common.rs @@ -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; @@ -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` /// @@ -563,7 +566,7 @@ pub fn check_extension_allowed( #[cfg(unix)] pub fn create_file_with_mode( path: &Path, - mode: Option, + mode: Option, create_new: bool, ) -> std::io::Result { use std::fs::OpenOptions; @@ -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)?; @@ -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) @@ -625,7 +628,7 @@ pub fn create_file_with_mode( #[cfg(not(unix))] pub fn create_file_with_mode( path: &Path, - _mode: Option, + _mode: Option, create_new: bool, ) -> std::io::Result { if create_new { @@ -720,7 +723,7 @@ pub fn create_file_with_mode( pub fn extract_file_with_permit( reader: &mut R, safe_path: &SafePath, - mode: Option, + mode: Option, _permit: QuotaPermit, dest: &DestDir, report: &mut ExtractionReport, @@ -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"); @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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 @@ -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 @@ -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 @@ -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"); @@ -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) }; diff --git a/crates/exarch-core/src/formats/compression.rs b/crates/exarch-core/src/formats/compression.rs index 003a3c4..f20d17d 100644 --- a/crates/exarch-core/src/formats/compression.rs +++ b/crates/exarch-core/src/formats/compression.rs @@ -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). /// diff --git a/crates/exarch-core/src/formats/detect.rs b/crates/exarch-core/src/formats/detect.rs index 17cc505..048551b 100644 --- a/crates/exarch-core/src/formats/detect.rs +++ b/crates/exarch-core/src/formats/detect.rs @@ -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, diff --git a/crates/exarch-core/src/formats/tar.rs b/crates/exarch-core/src/formats/tar.rs index 29facda..af5a3fa 100644 --- a/crates/exarch-core/src/formats/tar.rs +++ b/crates/exarch-core/src/formats/tar.rs @@ -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; @@ -288,7 +289,7 @@ impl TarArchive { fn extract_file( entry: &mut tar::Entry<'_, ER>, safe_path: &SafePath, - mode: Option, + mode: Option, permit: QuotaPermit, ctx: &mut ExtractionContext<'_, '_>, ) -> Result<()> { diff --git a/crates/exarch-core/src/formats/zip.rs b/crates/exarch-core/src/formats/zip.rs index 4361734..063dc43 100644 --- a/crates/exarch-core/src/formats/zip.rs +++ b/crates/exarch-core/src/formats/zip.rs @@ -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; @@ -453,7 +454,7 @@ impl ZipArchive { fn extract_file( zip_file: &mut zip::read::ZipFile<'_, R>, safe_path: &SafePath, - mode: Option, + mode: Option, permit: QuotaPermit, file_size: u64, ctx: &mut ZipExtractionContext<'_>, diff --git a/crates/exarch-core/src/inspection/report.rs b/crates/exarch-core/src/inspection/report.rs index ac090df..0a691ac 100644 --- a/crates/exarch-core/src/inspection/report.rs +++ b/crates/exarch-core/src/inspection/report.rs @@ -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, diff --git a/crates/exarch-core/src/inspection/verify.rs b/crates/exarch-core/src/inspection/verify.rs index 37447f9..303c02d 100644 --- a/crates/exarch-core/src/inspection/verify.rs +++ b/crates/exarch-core/src/inspection/verify.rs @@ -236,7 +236,7 @@ fn verify_entry( fn check_permissions(path: &Path, mode: u32, config: &SecurityConfig) -> Result<()> { let sanitized = sanitize_permissions(mode, config); - if sanitized == mode { + if sanitized.as_u32() == mode { Ok(()) } else { Err(ArchiveError::InvalidPermissions { diff --git a/crates/exarch-core/src/security/mod.rs b/crates/exarch-core/src/security/mod.rs index 51df69d..f9773f1 100644 --- a/crates/exarch-core/src/security/mod.rs +++ b/crates/exarch-core/src/security/mod.rs @@ -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")] diff --git a/crates/exarch-core/src/security/permissions.rs b/crates/exarch-core/src/security/permissions.rs index a004413..c402959 100644 --- a/crates/exarch-core/src/security/permissions.rs +++ b/crates/exarch-core/src/security/permissions.rs @@ -3,6 +3,48 @@ use crate::SecurityConfig; use crate::config::Validated; +/// A Unix permission mode that has already passed through +/// [`sanitize_permissions`]. +/// +/// This newtype closes the gap between "a raw mode read from an archive +/// header" and "a mode safe to apply to an extracted file": both are +/// otherwise a plain `u32`, so a caller could pass an unsanitized mode to +/// permission-setting code by mistake, with the invariant living only in a +/// doc comment. `SanitizedMode` can only be constructed by +/// [`sanitize_permissions`] — its single field is private, so external +/// callers (and other modules in this crate) cannot fabricate one that +/// skipped the setuid/setgid/world-writable stripping. +/// +/// # Examples +/// +/// ``` +/// # #[cfg(feature = "testing")] +/// # fn main() -> Result<(), Box> { +/// use exarch_core::SecurityConfig; +/// use exarch_core::security::sanitize_permissions; +/// +/// let config = SecurityConfig::default().validate()?; +/// +/// // Setuid bit is stripped +/// let sanitized = sanitize_permissions(0o4755, &config); +/// assert_eq!(sanitized.as_u32(), 0o755); +/// # Ok(()) +/// # } +/// # #[cfg(not(feature = "testing"))] +/// # fn main() {} +/// ``` +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SanitizedMode(u32); + +impl SanitizedMode { + /// Returns the sanitized mode as a raw Unix permission bitmask. + #[inline] + #[must_use] + pub fn as_u32(self) -> u32 { + self.0 + } +} + /// Sanitizes file permissions by stripping dangerous bits. /// /// This function removes security-sensitive permission bits that could @@ -12,36 +54,47 @@ use crate::config::Validated; /// - World-writable bit (0002): Stripped by default unless /// `allow_world_writable` is set /// +/// The [`SanitizedMode`] return type is the only way to obtain a sanitized +/// mode in this crate, so a caller cannot accidentally pass a raw, +/// unsanitized mode to internal permission-setting code such as +/// `create_file_with_mode`. +/// /// # Performance /// /// This is a pure computation with no I/O. Typical execution time: <10 ns. /// /// # Examples /// -/// ```ignore +/// ``` +/// # #[cfg(feature = "testing")] +/// # fn main() -> Result<(), Box> { /// use exarch_core::SecurityConfig; /// use exarch_core::security::sanitize_permissions; /// -/// let config = SecurityConfig::default(); +/// let config = SecurityConfig::default().validate()?; /// /// // Setuid bit is stripped /// let sanitized = sanitize_permissions(0o4755, &config); -/// assert_eq!(sanitized, 0o755); +/// assert_eq!(sanitized.as_u32(), 0o755); /// /// // Setgid bit is stripped /// let sanitized = sanitize_permissions(0o2755, &config); -/// assert_eq!(sanitized, 0o755); +/// assert_eq!(sanitized.as_u32(), 0o755); /// /// // Both setuid and setgid bits stripped /// let sanitized = sanitize_permissions(0o6755, &config); -/// assert_eq!(sanitized, 0o755); +/// assert_eq!(sanitized.as_u32(), 0o755); /// /// // World-writable bit is stripped by default /// let sanitized = sanitize_permissions(0o777, &config); -/// assert_eq!(sanitized, 0o775); +/// assert_eq!(sanitized.as_u32(), 0o775); +/// # Ok(()) +/// # } +/// # #[cfg(not(feature = "testing"))] +/// # fn main() {} /// ``` #[must_use] -pub fn sanitize_permissions(mode: u32, config: &SecurityConfig) -> u32 { +pub fn sanitize_permissions(mode: u32, config: &SecurityConfig) -> SanitizedMode { let mut sanitized = mode; // Strip setuid bit (04000) @@ -55,7 +108,7 @@ pub fn sanitize_permissions(mode: u32, config: &SecurityConfig) -> u3 sanitized &= !0o002; } - sanitized + SanitizedMode(sanitized) } #[cfg(test)] @@ -66,67 +119,67 @@ mod tests { #[test] fn test_sanitize_permissions_normal() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o644, &config), 0o644); + assert_eq!(sanitize_permissions(0o644, &config).as_u32(), 0o644); } #[test] fn test_sanitize_permissions_executable() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o755, &config), 0o755); + assert_eq!(sanitize_permissions(0o755, &config).as_u32(), 0o755); } #[test] fn test_sanitize_permissions_strip_setuid() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o4755, &config), 0o755); + assert_eq!(sanitize_permissions(0o4755, &config).as_u32(), 0o755); } #[test] fn test_sanitize_permissions_strip_setgid() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o2755, &config), 0o755); + assert_eq!(sanitize_permissions(0o2755, &config).as_u32(), 0o755); } #[test] fn test_sanitize_permissions_strip_both() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o6755, &config), 0o755); + assert_eq!(sanitize_permissions(0o6755, &config).as_u32(), 0o755); } #[test] fn test_sanitize_permissions_strip_world_writable() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o777, &config), 0o775); + assert_eq!(sanitize_permissions(0o777, &config).as_u32(), 0o775); } #[test] fn test_sanitize_permissions_world_readable_ok() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o644, &config), 0o644); + assert_eq!(sanitize_permissions(0o644, &config).as_u32(), 0o644); } #[test] fn test_sanitize_permissions_owner_writable_ok() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o600, &config), 0o600); + assert_eq!(sanitize_permissions(0o600, &config).as_u32(), 0o600); } #[test] fn test_sanitize_permissions_group_writable_ok() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o664, &config), 0o664); + assert_eq!(sanitize_permissions(0o664, &config).as_u32(), 0o664); } #[test] fn test_sanitize_permissions_edge_case_zero() { let config = SecurityConfig::default().validate().expect("valid config"); - assert_eq!(sanitize_permissions(0o000, &config), 0o000); + assert_eq!(sanitize_permissions(0o000, &config).as_u32(), 0o000); } #[test] fn test_sticky_bit_preservation() { let config = SecurityConfig::default().validate().expect("valid config"); - let sanitized = sanitize_permissions(0o1755, &config); + let sanitized = sanitize_permissions(0o1755, &config).as_u32(); assert_eq!(sanitized & 0o1000, 0o1000, "sticky bit should be preserved"); assert_eq!(sanitized, 0o1755, "full mode should be preserved"); } @@ -134,7 +187,7 @@ mod tests { #[test] fn test_sticky_bit_with_setuid_stripped() { let config = SecurityConfig::default().validate().expect("valid config"); - let sanitized = sanitize_permissions(0o7755, &config); + let sanitized = sanitize_permissions(0o7755, &config).as_u32(); assert_eq!(sanitized & 0o1000, 0o1000, "sticky bit should remain"); assert_eq!(sanitized & 0o4000, 0, "setuid should be stripped"); assert_eq!(sanitized & 0o2000, 0, "setgid should be stripped"); @@ -147,7 +200,7 @@ mod tests { let mut config = SecurityConfig::default(); config.allowed.world_writable = true; let config = config.validate().expect("valid config"); - let sanitized = sanitize_permissions(0o777, &config); + let sanitized = sanitize_permissions(0o777, &config).as_u32(); assert_eq!( sanitized & 0o002, 0o002, @@ -162,7 +215,7 @@ mod tests { fn test_world_writable_stripped_by_default() { let config = SecurityConfig::default().validate().expect("valid config"); assert_eq!( - sanitize_permissions(0o777, &config), + sanitize_permissions(0o777, &config).as_u32(), 0o775, "world-writable bit should be stripped by default" ); @@ -173,7 +226,7 @@ mod tests { let config = SecurityConfig::default().validate().expect("valid config"); // 0o666 = rw-rw-rw-, only other-write (0o002) should be stripped -> 0o664 assert_eq!( - sanitize_permissions(0o666, &config), + sanitize_permissions(0o666, &config).as_u32(), 0o664, "only world-writable bit should be stripped, not group-write" ); diff --git a/crates/exarch-core/src/security/validator.rs b/crates/exarch-core/src/security/validator.rs index ce35394..e340814 100644 --- a/crates/exarch-core/src/security/validator.rs +++ b/crates/exarch-core/src/security/validator.rs @@ -11,6 +11,7 @@ use crate::config::Validated; use crate::formats::common::DirCache; use crate::security::context::ValidationContext; use crate::security::hardlink::HardlinkTracker; +use crate::security::permissions::SanitizedMode; use crate::security::permissions::sanitize_permissions; use crate::security::quota::QuotaPermit; use crate::security::quota::QuotaTracker; @@ -34,7 +35,7 @@ use crate::types::SafeSymlink; pub struct ValidatedEntry { safe_path: SafePath, entry_type: ValidatedEntryType, - mode: Option, + mode: Option, } impl ValidatedEntry { @@ -46,7 +47,7 @@ impl ValidatedEntry { pub(crate) fn new( safe_path: SafePath, entry_type: ValidatedEntryType, - mode: Option, + mode: Option, ) -> Self { Self { safe_path, @@ -72,7 +73,7 @@ impl ValidatedEntry { /// Returns the sanitized file permissions, if applicable. #[inline] #[must_use] - pub fn mode(&self) -> Option { + pub fn mode(&self) -> Option { self.mode } @@ -87,7 +88,7 @@ impl ValidatedEntry { /// call this instead of `entry_type()`. #[inline] #[must_use] - pub(crate) fn into_parts(self) -> (SafePath, ValidatedEntryType, Option) { + pub(crate) fn into_parts(self) -> (SafePath, ValidatedEntryType, Option) { (self.safe_path, self.entry_type, self.mode) } } @@ -453,7 +454,7 @@ mod tests { let entry = result.unwrap(); assert_eq!(entry.safe_path.as_path(), Path::new("file.txt")); assert_matches!(entry.entry_type, ValidatedEntryType::File(_)); - assert_eq!(entry.mode, Some(0o644)); + assert_eq!(entry.mode.map(SanitizedMode::as_u32), Some(0o644)); } #[test] @@ -628,7 +629,7 @@ mod tests { assert!(result.is_ok()); let entry = result.unwrap(); - assert_eq!(entry.mode, Some(0o755)); // setuid stripped + assert_eq!(entry.mode.map(SanitizedMode::as_u32), Some(0o755)); // setuid stripped } #[test] diff --git a/crates/exarch-core/src/types/entry_type.rs b/crates/exarch-core/src/types/entry_type.rs index 5e706de..195b6be 100644 --- a/crates/exarch-core/src/types/entry_type.rs +++ b/crates/exarch-core/src/types/entry_type.rs @@ -20,7 +20,11 @@ use std::path::PathBuf; /// target: PathBuf::from("../target"), /// }; /// ``` +/// +/// `#[non_exhaustive]` so a future archive entry kind is not a breaking +/// change for downstream matches. #[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[non_exhaustive] pub enum EntryType { /// Regular file entry. File, diff --git a/crates/exarch-core/tests/ui/sanitized_mode_forge_via_tuple_struct.rs b/crates/exarch-core/tests/ui/sanitized_mode_forge_via_tuple_struct.rs new file mode 100644 index 0000000..63d2688 --- /dev/null +++ b/crates/exarch-core/tests/ui/sanitized_mode_forge_via_tuple_struct.rs @@ -0,0 +1,11 @@ +//! A `SanitizedMode` must not be assemblable from outside the crate via +//! tuple-struct call syntax. Its single field is private, so +//! `SanitizedMode(todo!())` cannot compile outside `exarch_core` — the only +//! producer of a real `SanitizedMode` is `sanitize_permissions`. + +use exarch_core::security::SanitizedMode; + +#[allow(unreachable_code)] +fn main() { + let _mode = SanitizedMode(todo!()); +} diff --git a/crates/exarch-core/tests/ui/sanitized_mode_forge_via_tuple_struct.stderr b/crates/exarch-core/tests/ui/sanitized_mode_forge_via_tuple_struct.stderr new file mode 100644 index 0000000..54258cd --- /dev/null +++ b/crates/exarch-core/tests/ui/sanitized_mode_forge_via_tuple_struct.stderr @@ -0,0 +1,11 @@ +error[E0423]: cannot initialize a tuple struct which contains private fields + --> tests/ui/sanitized_mode_forge_via_tuple_struct.rs:10:17 + | +10 | let _mode = SanitizedMode(todo!()); + | ^^^^^^^^^^^^^ + | +note: constructor is not visible here due to private fields + --> src/security/permissions.rs + | + | pub struct SanitizedMode(u32); + | ^^^ private field diff --git a/crates/exarch-node/src/error.rs b/crates/exarch-node/src/error.rs index e719baf..a14f254 100644 --- a/crates/exarch-node/src/error.rs +++ b/crates/exarch-node/src/error.rs @@ -98,6 +98,10 @@ pub fn convert_error(err: CoreError) -> Error { "QUOTA_EXCEEDED: quota exceeded: integer overflow in quota tracking", ); } + // Forward-compat: a variant added to QuotaResource after this + // match was written. #[non_exhaustive] requires this arm to + // compile against a newer exarch-core. + _ => msg.push_str("QUOTA_EXCEEDED: quota exceeded"), } Error::new(Status::GenericFailure, msg) } @@ -177,6 +181,13 @@ pub fn convert_error(err: CoreError) -> Error { ); Error::new(Status::GenericFailure, msg) } + // 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::new( + Status::GenericFailure, + "UNKNOWN: unrecognized archive error", + ), } } diff --git a/crates/exarch-python/src/error.rs b/crates/exarch-python/src/error.rs index 7c03ff8..3634fdb 100644 --- a/crates/exarch-python/src/error.rs +++ b/crates/exarch-python/src/error.rs @@ -81,6 +81,10 @@ pub fn convert_error(err: CoreError) -> PyErr { CoreQuotaResource::IntegerOverflow => { "quota exceeded: integer overflow in quota tracking".to_string() } + // Forward-compat: a variant added to QuotaResource after this + // match was written. #[non_exhaustive] requires this arm to + // compile against a newer exarch-core. + _ => "quota exceeded".to_string(), }; QuotaExceededError::new_err(msg) } @@ -133,6 +137,10 @@ pub fn convert_error(err: CoreError) -> PyErr { source_err }) } + // Forward-compat: a variant added to ArchiveError after this match was + // written. #[non_exhaustive] requires this arm to compile against a + // newer exarch-core. + _ => ArchiveError::new_err("unrecognized archive error"), } }