Skip to content

Commit 20299e7

Browse files
committed
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
1 parent 27c44b4 commit 20299e7

19 files changed

Lines changed: 216 additions & 54 deletions

File tree

CHANGELOG.md

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

2741
- **Bumped `sevenz-rust2` from 0.21.4 to 0.21.5, pulling in a transitive `lzma-rust2` bump from
2842
0.18.0 to 0.19.0 (#548)**: `sevenz-rust2` 0.21.5 batches AES-CBC block decryption, a 7z-extraction

crates/exarch-cli/src/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,10 @@ pub fn convert_extraction_error(
254254
archive.display(),
255255
)
256256
}
257+
// Forward-compat: a variant added to ArchiveError after this match was
258+
// written. #[non_exhaustive] requires this arm to compile against a
259+
// newer exarch-core; there is no more specific context to add.
260+
_ => format!("Error while processing '{}'", archive.display()),
257261
};
258262
anyhow::Error::from(err).context(context)
259263
}

crates/exarch-cli/src/output/json.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ fn extraction_error_kind(err: &ArchiveError) -> String {
3434
ArchiveError::UnknownFormat { .. } => "UnknownFormat",
3535
ArchiveError::InvalidConfiguration { .. } => "InvalidConfiguration",
3636
ArchiveError::PartialExtraction { source, .. } => return extraction_error_kind(source),
37+
// Forward-compat: a variant added to ArchiveError after this match was
38+
// written. #[non_exhaustive] requires this arm to compile against a
39+
// newer exarch-core. "Error" matches the generic fallback documented
40+
// for kinds that don't map to a known archive validation failure.
41+
_ => "Error",
3742
}
3843
.to_string()
3944
}

crates/exarch-core/src/error/types.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ use thiserror::Error;
77
pub type Result<T> = std::result::Result<T, ArchiveError>;
88

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

5660
/// Errors that can occur during archive operations (extraction, creation,
5761
/// listing, verification).
62+
///
63+
/// `#[non_exhaustive]` so a future error variant is not a breaking change
64+
/// for downstream matches.
5865
#[derive(Error, Debug)]
66+
#[non_exhaustive]
5967
pub enum ArchiveError {
6068
/// I/O operation failed.
6169
#[error("I/O error: {0}")]

crates/exarch-core/src/formats/common.rs

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ use crate::config::Validated;
3838
use crate::copy::CopyBuffer;
3939
use crate::copy::copy_with_buffer;
4040
use crate::error::QuotaResource;
41+
use crate::security::permissions::SanitizedMode;
4142
use crate::security::quota::QuotaPermit;
4243
use crate::types::DestDir;
4344
use crate::types::SafePath;
@@ -542,14 +543,16 @@ pub fn check_extension_allowed(
542543
/// - Strip sticky bit (0o1000) if required by security policy
543544
/// - Ensure world-writable permissions are only set if allowed
544545
///
545-
/// Mode sanitization MUST be performed by the caller (typically in the
546-
/// validation layer via `SecurityConfig::sanitize_mode()`). This function
547-
/// does NOT perform any sanitization and will apply the mode value directly.
546+
/// The [`SanitizedMode`] parameter type enforces mode sanitization at
547+
/// compile time: only
548+
/// [`sanitize_permissions`](crate::security::sanitize_permissions)
549+
/// can construct one, so a raw, unsanitized mode read from an archive header
550+
/// cannot reach this function by mistake.
548551
///
549552
/// # Arguments
550553
///
551554
/// * `path` - Path where file should be created
552-
/// * `mode` - Optional Unix file mode (must be pre-sanitized by caller)
555+
/// * `mode` - Optional pre-sanitized Unix file mode
553556
/// * `create_new` - If `true`, fail with `AlreadyExists` instead of truncating
554557
/// an existing file at `path`
555558
///
@@ -563,7 +566,7 @@ pub fn check_extension_allowed(
563566
#[cfg(unix)]
564567
pub fn create_file_with_mode(
565568
path: &Path,
566-
mode: Option<u32>,
569+
mode: Option<SanitizedMode>,
567570
create_new: bool,
568571
) -> std::io::Result<File> {
569572
use std::fs::OpenOptions;
@@ -585,7 +588,7 @@ pub fn create_file_with_mode(
585588

586589
if let Some(m) = mode {
587590
// Apply sanitized mode during open (already stripped setuid/setgid)
588-
opts.mode(m);
591+
opts.mode(m.as_u32());
589592
}
590593

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

604607
Ok(file)
@@ -625,7 +628,7 @@ pub fn create_file_with_mode(
625628
#[cfg(not(unix))]
626629
pub fn create_file_with_mode(
627630
path: &Path,
628-
_mode: Option<u32>,
631+
_mode: Option<SanitizedMode>,
629632
create_new: bool,
630633
) -> std::io::Result<File> {
631634
if create_new {
@@ -720,7 +723,7 @@ pub fn create_file_with_mode(
720723
pub fn extract_file_with_permit<R: Read>(
721724
reader: &mut R,
722725
safe_path: &SafePath,
723-
mode: Option<u32>,
726+
mode: Option<SanitizedMode>,
724727
_permit: QuotaPermit,
725728
dest: &DestDir,
726729
report: &mut ExtractionReport,
@@ -1079,12 +1082,22 @@ mod tests {
10791082
use crate::NoopProgress;
10801083
use crate::SecurityConfig;
10811084
use crate::copy::CopyBuffer;
1085+
use crate::security::permissions::sanitize_permissions;
10821086
use crate::security::quota::QuotaTracker;
10831087
use std::assert_matches;
10841088
use std::io::Cursor;
10851089
use std::path::PathBuf;
10861090
use tempfile::TempDir;
10871091

1092+
/// Builds a [`SanitizedMode`] for tests that don't otherwise need a
1093+
/// `SecurityConfig` in scope. None of the modes used across these tests
1094+
/// carry setuid/setgid/world-writable bits, so sanitizing with the
1095+
/// default config never changes the value.
1096+
fn sanitized(mode: u32) -> SanitizedMode {
1097+
let config = SecurityConfig::default().validate().expect("valid config");
1098+
sanitize_permissions(mode, &config)
1099+
}
1100+
10881101
#[test]
10891102
fn test_extract_file_with_permit_integer_overflow_check() {
10901103
let temp = TempDir::new().expect("failed to create temp dir");
@@ -1111,7 +1124,7 @@ mod tests {
11111124
let result = extract_file_with_permit(
11121125
&mut reader,
11131126
&safe_path,
1114-
Some(0o644),
1127+
Some(sanitized(0o644)),
11151128
permit,
11161129
&dest,
11171130
&mut report,
@@ -1175,7 +1188,7 @@ mod tests {
11751188
let result = extract_file_with_permit(
11761189
&mut reader,
11771190
&safe_path,
1178-
Some(0o644),
1191+
Some(sanitized(0o644)),
11791192
permit,
11801193
&dest,
11811194
&mut report,
@@ -1224,7 +1237,7 @@ mod tests {
12241237
let result = extract_file_with_permit(
12251238
&mut reader,
12261239
&safe_path,
1227-
Some(0o644),
1240+
Some(sanitized(0o644)),
12281241
permit,
12291242
&dest,
12301243
&mut report,
@@ -1270,7 +1283,7 @@ mod tests {
12701283
let result = extract_file_with_permit(
12711284
&mut reader,
12721285
&safe_path,
1273-
Some(0o644),
1286+
Some(sanitized(0o644)),
12741287
permit,
12751288
&dest,
12761289
&mut report,
@@ -1327,7 +1340,7 @@ mod tests {
13271340
let result = extract_file_with_permit(
13281341
&mut reader,
13291342
&safe_path,
1330-
Some(0o644),
1343+
Some(sanitized(0o644)),
13311344
permit,
13321345
&dest,
13331346
&mut report,
@@ -1557,8 +1570,8 @@ mod tests {
15571570
let file_path = temp.path().join("test_0o644.txt");
15581571

15591572
// Create file with mode 0o644
1560-
let file =
1561-
create_file_with_mode(&file_path, Some(0o644), false).expect("should create file");
1573+
let file = create_file_with_mode(&file_path, Some(sanitized(0o644)), false)
1574+
.expect("should create file");
15621575
drop(file);
15631576

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

15881601
// Create file with mode 0o755
1589-
let file =
1590-
create_file_with_mode(&file_path, Some(0o755), false).expect("should create file");
1602+
let file = create_file_with_mode(&file_path, Some(sanitized(0o755)), false)
1603+
.expect("should create file");
15911604
drop(file);
15921605

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

16171630
// Create file with mode 0o600
1618-
let file =
1619-
create_file_with_mode(&file_path, Some(0o600), false).expect("should create file");
1631+
let file = create_file_with_mode(&file_path, Some(sanitized(0o600)), false)
1632+
.expect("should create file");
16201633
drop(file);
16211634

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

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

1766-
let result = create_file_with_mode(&file_path, Some(0o755), false);
1779+
let result = create_file_with_mode(&file_path, Some(sanitized(0o755)), false);
17671780

17681781
// Restore previous umask unconditionally before any assert.
17691782
unsafe { libc::umask(previous_umask) };

crates/exarch-core/src/formats/compression.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@
3636
/// let best_codec = CompressionCodec::Xz; // Best compression ratio
3737
/// let modern_codec = CompressionCodec::Zstd; // Modern balanced approach
3838
/// ```
39+
///
40+
/// `#[non_exhaustive]` so support for a new compression codec is not a
41+
/// breaking change for downstream matches.
3942
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
43+
#[non_exhaustive]
4044
pub enum CompressionCodec {
4145
/// Gzip compression (deflate algorithm).
4246
///

crates/exarch-core/src/formats/detect.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,11 @@ pub(crate) fn is_zip_family_alias(ext: &str) -> bool {
5151
}
5252

5353
/// Supported archive formats.
54+
///
55+
/// `#[non_exhaustive]` so support for a new archive format is not a
56+
/// breaking change for downstream matches.
5457
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
58+
#[non_exhaustive]
5559
pub enum ArchiveType {
5660
/// Tar archive (uncompressed).
5761
Tar,

crates/exarch-core/src/formats/tar.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ use crate::Result;
123123
use crate::SecurityConfig;
124124
use crate::config::Validated;
125125
use crate::copy::CopyBuffer;
126+
use crate::security::permissions::SanitizedMode;
126127
use crate::security::quota::QuotaPermit;
127128
use crate::security::validator::EntryValidator;
128129
use crate::security::validator::ValidatedEntryType;
@@ -288,7 +289,7 @@ impl<R: Read> TarArchive<R> {
288289
fn extract_file<ER: Read>(
289290
entry: &mut tar::Entry<'_, ER>,
290291
safe_path: &SafePath,
291-
mode: Option<u32>,
292+
mode: Option<SanitizedMode>,
292293
permit: QuotaPermit,
293294
ctx: &mut ExtractionContext<'_, '_>,
294295
) -> Result<()> {

crates/exarch-core/src/formats/zip.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ use crate::SecurityConfig;
135135
use crate::config::Validated;
136136
use crate::copy::CopyBuffer;
137137
use crate::security::EntryValidator;
138+
use crate::security::permissions::SanitizedMode;
138139
use crate::security::quota::QuotaPermit;
139140
use crate::security::validator::ValidatedEntryType;
140141
use crate::types::DestDir;
@@ -453,7 +454,7 @@ impl<R: Read + Seek> ZipArchive<R> {
453454
fn extract_file(
454455
zip_file: &mut zip::read::ZipFile<'_, R>,
455456
safe_path: &SafePath,
456-
mode: Option<u32>,
457+
mode: Option<SanitizedMode>,
457458
permit: QuotaPermit,
458459
file_size: u64,
459460
ctx: &mut ZipExtractionContext<'_>,

crates/exarch-core/src/inspection/report.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,11 @@ impl std::fmt::Display for IssueSeverity {
292292
}
293293

294294
/// Issue categories (maps to security checks).
295+
///
296+
/// `#[non_exhaustive]` so a future security check is not a breaking change
297+
/// for downstream matches.
295298
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
299+
#[non_exhaustive]
296300
pub enum IssueCategory {
297301
/// Path traversal attack
298302
PathTraversal,

0 commit comments

Comments
 (0)