Skip to content

Commit b7ae624

Browse files
committed
Bugfix: an encrypted 7z now refuses honestly as "not supported" instead of "damaged archive"
With `sevenz-rust2`'s `aes256` feature off (it conflicts with `smb2`'s pinned `aes` pre-release), an AES-encrypted 7z has an unrecognized coder, so the crate returns `UnsupportedCompressionMethod("AES256_SHA256")` — at open (`ArchiveReader::new`) for a header-encrypted archive, at decode (`for_each_entries`) for a data-encrypted one. `map_sevenz_err` flattened EVERY sevenz error to `ArchiveError::Corrupt`, so an intact encrypted 7z surfaced to the user as a DAMAGED archive. - `map_sevenz_err` now classifies by typed enum variant: the encryption/unknown-coder family (`UnsupportedCompressionMethod`, `Unsupported`, `ExternalUnsupported`, `UnsupportedVersion`, and the `aes256`-on-only `PasswordRequired` / `MaybeBadPassword`) maps to `ArchiveError::Unsupported` → `VolumeError::NotSupported`, the honest "can't open this kind" — never `Corrupt` ("damaged"), never `Encrypted` (a password prompt a 7z read can never satisfy here). A memory-limit refusal maps to `TooLarge`; byte-source I/O keeps the io-kind classifier; genuinely broken/truncated structure stays `Corrupt`. - Pinned by `sevenz_test.rs`: unit tests over constructed `sevenz_rust2::Error` values (the crate can't encrypt with `aes256` off, so an in-memory encrypted fixture isn't buildable and a `7z`-CLI fixture would be absent from CI). The exact error shape was empirically confirmed against real `7z`-produced `-mhe=on` / `-mhe=off` fixtures (sevenz-rust2 0.21.2). - Docs corrected to current state: the `sevenz.rs` module + function comments, `read/CLAUDE.md`, `read/DETAILS.md` § Decryption and § tar/7z, and the `archive-browsing-polish` spec §2 — which overstated 7z AES as a "one-line flip"; it needs a per-archive password threaded through `parse` and every `open_read` / `stream_subtree` re-open, not just the feature flag.
1 parent e9fa9de commit b7ae624

5 files changed

Lines changed: 168 additions & 14 deletions

File tree

apps/desktop/src-tauri/src/file_system/volume/backends/archive/read/CLAUDE.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ planning, reorganizing, or advising.
3939
routed through the `zip` crate's `by_index_decrypt` by CENTRAL-DIRECTORY ORDINAL (rc-zip parses but can't decrypt; the
4040
ordinals align — pinned). No password ⇒ `Encrypted`; wrong ⇒ `WrongPassword` (ZipCrypto's wrong-password may surface
4141
late as an end-of-stream CRC, mapped by io-kind not message). WinZip AES and 7z AES are deferred (`aes` crate conflicts
42-
with `smb2`) and return a typed refusal. Filename encoding is rc-zip's job — consume the decoded `entry.name`.
42+
with `smb2`) and refuse honestly as `Unsupported` (→ `NotSupported`; never a "damaged archive", never a password
43+
prompt that can't succeed) — for 7z via `sevenz.rs::map_sevenz_err`. Filename encoding is rc-zip's job — consume the
44+
decoded `entry.name`.
4345
- **The index cache key is `(path, size, mtime)`** (external edits auto-invalidate); `index_for_local` is blocking, call
4446
it from `spawn_blocking`.
4547
- **Two DoS caps bound the synthetic tree**: per-entry depth (`name::MAX_COMPONENT_DEPTH`, over-deep entries quarantine)

apps/desktop/src-tauri/src/file_system/volume/backends/archive/read/DETAILS.md

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,10 @@ the per-entry decrypt engine.
157157
feature. **WinZip AES** (the `Method::Aex` marker) would need the `zip` crate's `aes-crypto` feature, and **7z AES**
158158
the `sevenz-rust2` `aes256` feature — both pull `aes ^0.9` (stable 0.9.1), which CONFLICTS with `smb2`'s pinned
159159
`aes =0.9.0-rc.4` (its SMB3 AEAD stack), so Cargo can't unify. Until the `aes` versions align, an AES zip entry returns
160-
a typed `Unsupported` (honest — not a password prompt that could never succeed) and encrypted 7z stays a typed refusal.
161-
Flipping the two features on (plus the AES branch in `zip::open_read`, already stubbed) is the whole AES follow-up.
160+
a typed `Unsupported` (honest — not a password prompt that could never succeed), and an encrypted 7z refuses the same
161+
way: `sevenz.rs::map_sevenz_err` classifies the unrecognized `AES256_SHA256` coder as `Unsupported`.
162+
Flipping zip's `aes-crypto` on (plus the AES branch in `zip::open_read`, already stubbed) is the zip AES follow-up; 7z
163+
AES needs more than the feature flag (see below).
162164
- **Wrong-password detection.** zip AES verifies a 2-byte password check at open (`InvalidPassword``WrongPassword`,
163165
instant) — relevant once AES lands. ZipCrypto checks a single byte at open, so ~1/256 of wrong passwords slip through
164166
and surface LATE as an end-of-stream CRC mismatch: the `zip` crate returns `io::ErrorKind::InvalidData` ("Invalid
@@ -205,8 +207,16 @@ at offset 0 — a plain tar's `ustar` at offset 257, so the confirm reads a 512-
205207
streaming `.xz` reader `lzma-rs` lacks), zstd → `ruzstd`'s `StreamingDecoder`. Each handles concatenated members
206208
(`gzip -c a b`). tar parsing/streaming rides the `tar` crate over a `SourceReader` (a `Read`+`Seek` cursor on the byte
207209
source); 7z uses `sevenz-rust2`'s `ArchiveReader` (needs `Read`+`Seek`, hence `SourceReader`). LZMA/LZMA2 (the common 7z
208-
codec) is built into `sevenz-rust2`; `bzip2`/`deflate`/`ppmd` are feature-enabled. `aes256` is OFF, so an encrypted 7z
209-
surfaces a typed error rather than being decrypted (matches zip's "encrypted → reject").
210+
codec) is built into `sevenz-rust2`; `bzip2`/`deflate`/`ppmd` are feature-enabled. `aes256` is OFF, so an AES-encrypted
211+
7z has an unrecognized coder: `sevenz-rust2` returns `UnsupportedCompressionMethod("AES256_SHA256")` — at open
212+
(`ArchiveReader::new`) for a header-encrypted archive (`7z -mhe=on`, browsing itself refused), at decode
213+
(`for_each_entries`) for a data-encrypted one (`7z -mhe=off`, browsing works, extraction refused). `map_sevenz_err`
214+
classifies both (plus the `aes256`-on-only `PasswordRequired` / `MaybeBadPassword`) as `Unsupported``NotSupported`:
215+
the honest "can't open this kind", NOT `Corrupt` ("damaged") and NOT `Encrypted` (a password prompt a 7z read can never
216+
satisfy here). Pinned by `sevenz_test.rs`. (Verified on sevenz-rust2 0.21.2, `aes256` off, against real `7z` fixtures,
217+
2026-07-08.) Enabling it isn't a one-line feature flip: `sevenz-rust2` needs the password at `ArchiveReader::new` time
218+
(a `Password`, not `Password::empty()`), so a real 7z-AES path must thread a per-archive password through `parse` AND
219+
every `open_read` / `stream_subtree` re-open — the parse/read plumbing zip's ZipCrypto path already has, which 7z lacks.
210220

211221
**Access class — the sequential trap (`ArchiveFormat::is_sequential`, `Volume::extraction_is_sequential`).** A plain
212222
`.tar` is RANDOM-access: `TarStore` records each member's data offset, so `open_read` seeks and streams the member's

apps/desktop/src-tauri/src/file_system/volume/backends/archive/read/sevenz.rs

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111
//!
1212
//! Encryption is out of scope: the `aes256` feature is off (it would pull an
1313
//! `aes` version conflicting with `smb2`'s pinned pre-release; see the archive
14-
//! `DETAILS.md`), so an AES-encrypted 7z surfaces a typed error rather than being
15-
//! decrypted — matching the deferral of WinZip AES zip.
14+
//! `DETAILS.md`), so an AES-encrypted 7z refuses honestly as `Unsupported` (a
15+
//! "can't open this kind" the user sees, never a "damaged archive" or a password
16+
//! prompt) rather than being decrypted. The classification lives in
17+
//! [`map_sevenz_err`]; matching the deferral of WinZip AES zip.
1618
1719
use std::collections::HashMap;
1820
use std::sync::Arc;
@@ -208,9 +210,58 @@ pub(super) fn stream_subtree(source: Arc<dyn ArchiveByteSource>, mut wanted: Has
208210
}
209211
}
210212

211-
/// Maps a `sevenz-rust2` error to a typed [`ArchiveError`]. Encryption without
212-
/// the `aes256` feature, and any unknown codec, surface as `Unsupported`; a
213-
/// malformed header as `Corrupt`.
213+
/// Maps a `sevenz-rust2` error to a typed [`ArchiveError`], classifying by enum
214+
/// variant (never message text, per `no-string-matching`).
215+
///
216+
/// The load-bearing case is encryption. We build `sevenz-rust2` with the
217+
/// `aes256` feature OFF (its `aes` crate conflicts with `smb2`'s pin; see the
218+
/// archive `DETAILS.md`), so an AES-encrypted 7z has an unrecognized coder and
219+
/// the crate returns `UnsupportedCompressionMethod("AES256_SHA256")` — for a
220+
/// header-encrypted archive at open (`ArchiveReader::new`), for a data-encrypted
221+
/// one at decode (`for_each_entries`). Both must land on [`ArchiveError::Unsupported`]
222+
/// (→ `VolumeError::NotSupported`, the honest "can't open this kind"), NOT
223+
/// [`ArchiveError::Corrupt`] (which reads to the user as a DAMAGED archive) and NOT
224+
/// [`ArchiveError::Encrypted`] (which prompts for a password a 7z read can never
225+
/// satisfy here). A genuinely-unknown codec produces the same variant and is
226+
/// honestly "unsupported" too, so no AES-specific coder-id check is needed.
227+
/// (Verified on sevenz-rust2 0.21.2, `aes256` off, against real `7z`-produced
228+
/// `-mhe=on`/`-mhe=off` fixtures, 2026-07-08.)
229+
///
230+
/// The `PasswordRequired` / `MaybeBadPassword` variants only arise with `aes256`
231+
/// ON; they're mapped to `Unsupported` too so encryption never reads as damaged
232+
/// if the feature is ever enabled without a matching password-threading path.
214233
fn map_sevenz_err(err: sevenz_rust2::Error) -> ArchiveError {
215-
ArchiveError::Corrupt(format!("7z: {err}"))
234+
use sevenz_rust2::Error as E;
235+
match err {
236+
// Encryption (our `aes256`-off build) and any unknown/unsupported coder:
237+
// an honest "can't serve this kind", never "damaged".
238+
E::UnsupportedCompressionMethod(method) => ArchiveError::Unsupported(format!("7z coder: {method}")),
239+
E::Unsupported(msg) => ArchiveError::Unsupported(format!("7z: {msg}")),
240+
E::ExternalUnsupported => ArchiveError::Unsupported("7z uses an unsupported external coder".to_string()),
241+
E::UnsupportedVersion { major, minor } => ArchiveError::Unsupported(format!("7z format version {major}.{minor}")),
242+
E::PasswordRequired | E::MaybeBadPassword(_) => ArchiveError::Unsupported("7z is encrypted".to_string()),
243+
// A memory-limit refusal is a resource cap, not damage: reuse the tree-size
244+
// rejection so it never reads as "damaged".
245+
E::MaxMemLimited { max_kb, actaul_kb } => {
246+
ArchiveError::TooLarge(format!("7z needs {actaul_kb} KB to decode, over the {max_kb} KB limit"))
247+
}
248+
// Underlying byte-source I/O: reuse the io-kind classifier (UnexpectedEof ⇒ Corrupt).
249+
E::Io(io, _) | E::FileOpen(io, _) => ArchiveError::from(io),
250+
// Everything else is a structurally broken or truncated archive. `err` is
251+
// still owned here (every sub-pattern binds nothing), so format it whole.
252+
E::BadSignature(_)
253+
| E::ChecksumVerificationFailed
254+
| E::NextHeaderCrcMismatch
255+
| E::BadTerminatedStreamsInfo(_)
256+
| E::BadTerminatedUnpackInfo
257+
| E::BadTerminatedPackInfo(_)
258+
| E::BadTerminatedSubStreamsInfo
259+
| E::BadTerminatedHeader(_)
260+
| E::Other(_)
261+
| E::FileNotFound => ArchiveError::Corrupt(format!("7z: {err}")),
262+
}
216263
}
264+
265+
#[cfg(test)]
266+
#[path = "sevenz_test.rs"]
267+
mod sevenz_test;
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
//! Unit tests for [`super::map_sevenz_err`], the sevenz-rust2 → [`ArchiveError`]
2+
//! classifier.
3+
//!
4+
//! The encryption cases are the point: with the `aes256` feature off (our build),
5+
//! a real `7z`-produced encrypted archive makes sevenz-rust2 return
6+
//! `UnsupportedCompressionMethod("AES256_SHA256")` — at `ArchiveReader::new` for a
7+
//! header-encrypted archive (`7z -mhe=on`), at `for_each_entries` for a
8+
//! data-encrypted one (`7z -mhe=off`). Both shapes must classify as `Unsupported`
9+
//! so the user gets the honest "can't open this kind" experience, never "damaged
10+
//! archive". (Empirically confirmed on sevenz-rust2 0.21.2 against both fixtures,
11+
//! 2026-07-08.) We pin the predicate on constructed error values rather than a
12+
//! checked-in encrypted blob: the crate can't encrypt with `aes256` off, so an
13+
//! in-memory encrypted fixture isn't buildable, and shelling out to `7z` would
14+
//! make the test depend on a tool absent from CI.
15+
16+
use sevenz_rust2::Error as E;
17+
18+
use super::{ArchiveError, map_sevenz_err};
19+
20+
#[test]
21+
fn encrypted_7z_classifies_as_unsupported_not_corrupt() {
22+
// The exact error a real AES-encrypted 7z yields in our `aes256`-off build,
23+
// for BOTH the header-encrypted (open-time) and data-encrypted (decode-time)
24+
// shapes.
25+
let mapped = map_sevenz_err(E::UnsupportedCompressionMethod("AES256_SHA256".to_string()));
26+
assert!(
27+
matches!(mapped, ArchiveError::Unsupported(_)),
28+
"an encrypted 7z must refuse as Unsupported (honest 'can't open this kind'), got {mapped:?}"
29+
);
30+
// Guard the two failure modes the honesty fix exists to prevent:
31+
assert!(
32+
!matches!(mapped, ArchiveError::Corrupt(_)),
33+
"must not read as a damaged archive"
34+
);
35+
assert!(
36+
!matches!(mapped, ArchiveError::Encrypted | ArchiveError::WrongPassword),
37+
"must not trigger a password prompt a 7z read can never satisfy here"
38+
);
39+
}
40+
41+
#[test]
42+
fn unknown_7z_codec_also_unsupported() {
43+
let mapped = map_sevenz_err(E::UnsupportedCompressionMethod("SOME_FUTURE_CODEC".to_string()));
44+
assert!(matches!(mapped, ArchiveError::Unsupported(_)), "got {mapped:?}");
45+
}
46+
47+
#[test]
48+
fn aes256_on_password_shapes_map_to_unsupported() {
49+
// These only arise with the `aes256` feature ON; mapped defensively so
50+
// encryption never reads as damaged if the feature is ever enabled.
51+
assert!(matches!(map_sevenz_err(E::PasswordRequired), ArchiveError::Unsupported(_)));
52+
let io = std::io::Error::new(std::io::ErrorKind::InvalidData, "bad pw");
53+
assert!(matches!(map_sevenz_err(E::MaybeBadPassword(io)), ArchiveError::Unsupported(_)));
54+
}
55+
56+
#[test]
57+
fn broken_7z_structure_still_reads_as_corrupt() {
58+
// The fix must not over-reach: a genuinely damaged archive stays Corrupt.
59+
assert!(matches!(map_sevenz_err(E::NextHeaderCrcMismatch), ArchiveError::Corrupt(_)));
60+
assert!(matches!(map_sevenz_err(E::ChecksumVerificationFailed), ArchiveError::Corrupt(_)));
61+
assert!(matches!(map_sevenz_err(E::BadSignature([0; 6])), ArchiveError::Corrupt(_)));
62+
}
63+
64+
#[test]
65+
fn memory_limit_maps_to_too_large() {
66+
let mapped = map_sevenz_err(E::MaxMemLimited {
67+
max_kb: 1024,
68+
actaul_kb: 4096,
69+
});
70+
assert!(matches!(mapped, ArchiveError::TooLarge(_)), "got {mapped:?}");
71+
}
72+
73+
#[test]
74+
fn byte_source_io_classifies_by_io_kind() {
75+
// A truncated stream (UnexpectedEof) is a damaged archive, not a live fault.
76+
let eof = std::io::Error::new(std::io::ErrorKind::UnexpectedEof, "short read");
77+
assert!(matches!(map_sevenz_err(E::Io(eof, "".into())), ArchiveError::Corrupt(_)));
78+
// A real I/O fault stays Io.
79+
let denied = std::io::Error::new(std::io::ErrorKind::PermissionDenied, "denied");
80+
assert!(matches!(map_sevenz_err(E::Io(denied, "".into())), ArchiveError::Io(_)));
81+
}

docs/specs/later/archive-browsing-polish.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,19 @@ Backend half (decrypt in the read path, the typed `ArchiveNeedsPassword` signal,
3030
`DETAILS.md` § "Archive-password prompt".
3131

3232
**WinZip AES zip and 7z AES still deferred.** Enabling the `aes` crate (zip `aes-crypto` / sevenz `aes256`) pulls stable
33-
`aes 0.9.1`, which conflicts with `smb2`'s pinned `aes =0.9.0-rc.4` (its SMB3 AEAD stack) — Cargo can't unify. An AES
34-
entry returns a typed `Unsupported` (honest, not a prompt that can't succeed); the AES branch is stubbed for a one-line
35-
flip once the versions align.
33+
`aes 0.9.1`, which conflicts with `smb2`'s pinned `aes =0.9.0-rc.4` (its SMB3 AEAD stack) — Cargo can't unify. Both AES
34+
kinds refuse honestly as `Unsupported` today (never "damaged", never a prompt that can't succeed): zip via the stubbed
35+
AES branch in `zip::open_read`, 7z via `sevenz.rs::map_sevenz_err` (the unrecognized `AES256_SHA256` coder). The two
36+
follow-ups differ in size once the `aes` versions align:
37+
38+
- **WinZip AES zip is close to a one-line flip:** turn on `aes-crypto` and fill the already-stubbed AES branch — the
39+
password plumbing (per-archive storage, prompt, re-dispatch, wrong-password detection) is the same ZipCrypto path,
40+
already shipped.
41+
- **7z AES is NOT a one-line flip.** Beyond the `aes256` feature, `sevenz-rust2` wants the password at
42+
`ArchiveReader::new` time, so a real 7z-AES path must thread a per-archive password through `read/sevenz.rs`'s `parse`
43+
AND every `open_read` / `stream_subtree` re-open (each currently passes `Password::empty()`), then surface
44+
`Encrypted` / `WrongPassword` from `map_sevenz_err` instead of `Unsupported`. That's new parse/read plumbing, not a
45+
flag.
3646

3747
## 3. M-append: fast in-place zip edits (perf, research spike)
3848

0 commit comments

Comments
 (0)