From 2c101abdbd26433ebe23c8d676e31e2c20968e86 Mon Sep 17 00:00:00 2001
From: ddidderr <ddidderr@paul.network>
Date: Sat, 2 May 2026 21:29:28 +0200
Subject: [PATCH] fix(cli): reject `-j 0` instead of silently falling through
 to serial
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`--threads` was an `Option<usize>` with no value-parser bound. Passing
`-j 0` slipped past clap and reached the dispatch in `crypto::encrypt`
/ `decrypt`, where the `threads > 1` check evaluates to false for 0 and
the call quietly fell through to the serial path. The user thought
they had asked for "no worker threads" and got something else instead;
either way, 0 workers is not a meaningful configuration.

Switch the field to `Option<u32>` and add `value_parser!(u32).range(1..)`
so clap rejects `-j 0` at parse time with a usage error. Cast to
`usize` at the single use site. Using `u32` rather than `usize` avoids
shipping a host-pointer-width-dependent CLI surface; thread counts well
past 4 billion are not a thing we need to plan for.

Test plan:
  - New integration test `rejects_zero_threads` invokes the binary
    with `-j 0` and asserts non-zero exit.
  - Existing `roundtrip_multi_threaded` (`-j 4`) still passes, so the
    range bound has not broken normal usage.

Refs: external review (GLM51 #1; Gemini #3 misdescribed the symptom
as "silent corruption" — verified the actual behaviour was a fall-
through to the serial path, not output corruption, but the fix is
the same).
---
 src/main.rs        |  6 +++---
 tests/roundtrip.rs | 21 +++++++++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index a06b81e..d55abdb 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -67,8 +67,8 @@ struct Cli {
 
     /// Number of worker threads for AEAD work. Defaults to the number of
     /// available CPUs. Set to 1 for fully serial encrypt/decrypt.
-    #[clap(short = 'j', long)]
-    threads: Option<usize>,
+    #[clap(short = 'j', long, value_parser = clap::value_parser!(u32).range(1..))]
+    threads: Option<u32>,
 
     /// Random-access decrypt: byte offset of the slice to read.
     /// Requires `--decrypt`, an `--input-file` whose header has the
@@ -175,7 +175,7 @@ fn run(mut cli: Cli) -> Result<(), FcryError> {
     let argon_memory = cli.argon_memory;
     let argon_passes = cli.argon_passes;
     let argon_parallelism = cli.argon_parallelism;
-    let threads = cli.threads.unwrap_or_else(|| {
+    let threads = cli.threads.map(|n| n as usize).unwrap_or_else(|| {
         std::thread::available_parallelism()
             .map(|n| n.get())
             .unwrap_or(1)
diff --git a/tests/roundtrip.rs b/tests/roundtrip.rs
index db87514..58e7349 100644
--- a/tests/roundtrip.rs
+++ b/tests/roundtrip.rs
@@ -723,6 +723,27 @@ fn random_access_tampered_length_fails() {
     );
 }
 
+#[test]
+fn rejects_zero_threads() {
+    // -j 0 is almost certainly a user mistake. Clap should reject it before
+    // we ever reach the pipeline.
+    let dir = TempDir::new().unwrap();
+    let plain = dir.path().join("p.bin");
+    fs::write(&plain, b"hello").unwrap();
+    let out = fcry()
+        .arg("-i")
+        .arg(&plain)
+        .arg("-o")
+        .arg(dir.path().join("c.bin"))
+        .arg("--raw-key")
+        .arg(KEY_STR)
+        .arg("-j")
+        .arg("0")
+        .output()
+        .unwrap();
+    assert!(!out.status.success(), "-j 0 should be rejected");
+}
+
 #[test]
 fn header_chunk_size_is_authoritative_on_decrypt() {
     // Encrypt with a non-default chunk size; decrypt without specifying one.