Skip to content

Commit 8f59fcb

Browse files
committed
fix: Bypass validation in exclusive groups
1 parent 43c3b42 commit 8f59fcb

File tree

2 files changed

+77
-0
lines changed

2 files changed

+77
-0
lines changed

clap_builder/src/parser/validator.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,16 @@ impl<'cmd> Validator<'cmd> {
343343

344344
fn is_missing_required_ok(&self, a: &Arg, conflicts: &Conflicts) -> bool {
345345
debug!("Validator::is_missing_required_ok: {}", a.get_id());
346+
347+
// If this argument is conditionally required (i.e., required by other present arguments
348+
// through the 'requires' relationship), it's NOT OK for it to be missing, even if it
349+
// conflicts with other arguments. However, directly required arguments (marked as
350+
// required(true)) can still be bypassed by conflicts.
351+
if self.required.contains(a.get_id()) && !a.is_required_set() {
352+
debug!("Validator::is_missing_required_ok: false (conditionally required)");
353+
return false;
354+
}
355+
346356
if !conflicts.gather_conflicts(self.cmd, a.get_id()).is_empty() {
347357
debug!("Validator::is_missing_required_ok: true (self)");
348358
return true;

tests/builder/issue_4707.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,56 @@
55
/// These tests verify that the requires validation works correctly.
66
use clap::{Arg, ArgAction, ArgGroup, Command, error::ErrorKind};
77

8+
#[cfg(feature = "derive")]
9+
mod derive_tests {
10+
use clap::{ArgGroup, Parser};
11+
12+
#[derive(Parser, Debug)]
13+
#[clap(group = ArgGroup::new("command").multiple(false))]
14+
struct Args {
15+
#[clap(long, group = "command")]
16+
read: bool,
17+
18+
#[clap(long, group = "command")]
19+
write: bool,
20+
21+
#[clap(long, requires = "read")]
22+
show_hex: bool,
23+
}
24+
25+
#[test]
26+
fn issue_4707_original_derive_example() {
27+
// This is the exact example from the GitHub issue
28+
// It should fail when --show-hex is used without --read
29+
let result = Args::try_parse_from(["test", "--show-hex"]);
30+
31+
assert!(result.is_err(), "Should fail because --show-hex requires --read");
32+
assert_eq!(result.unwrap_err().kind(), clap::error::ErrorKind::MissingRequiredArgument);
33+
}
34+
35+
#[test]
36+
fn issue_4707_derive_example_with_write() {
37+
// Test the problematic case: using --write and --show-hex together
38+
// This should fail because --show-hex requires --read, but --write and --read are mutually exclusive
39+
let result = Args::try_parse_from(["test", "--write", "--show-hex"]);
40+
41+
assert!(result.is_err(), "Should fail because --show-hex requires --read but --write and --read are mutually exclusive");
42+
assert_eq!(result.unwrap_err().kind(), clap::error::ErrorKind::MissingRequiredArgument);
43+
}
44+
45+
#[test]
46+
fn issue_4707_derive_example_valid_case() {
47+
// This should succeed when --read and --show-hex are used together
48+
let result = Args::try_parse_from(["test", "--read", "--show-hex"]);
49+
50+
assert!(result.is_ok(), "Should succeed when --read and --show-hex are used together");
51+
let args = result.unwrap();
52+
assert!(args.read);
53+
assert!(args.show_hex);
54+
assert!(!args.write);
55+
}
56+
}
57+
858
#[test]
959
fn issue_4707_requires_should_be_validated_when_args_are_in_group() {
1060
// This test ensures that `requires` validation is NOT bypassed
@@ -92,3 +142,20 @@ fn issue_4707_complex_interaction_test() {
92142
let result3 = cmd.clone().try_get_matches_from(vec!["test", "-v", "-o"]);
93143
assert!(result3.is_ok(), "Should succeed when dependency is provided");
94144
}
145+
146+
#[test]
147+
fn issue_4707_exact_github_example_builder() {
148+
// This reproduces the exact case from the GitHub issue using clap builder instead of derive
149+
let cmd = Command::new("test")
150+
.arg(Arg::new("read").long("read").action(ArgAction::SetTrue).group("command"))
151+
.arg(Arg::new("write").long("write").action(ArgAction::SetTrue).group("command"))
152+
.arg(Arg::new("show_hex").long("show-hex").action(ArgAction::SetTrue).requires("read"))
153+
.group(ArgGroup::new("command").multiple(false));
154+
155+
// This exact case should fail: --write --show-hex
156+
// Because --show-hex requires --read, but --read and --write are mutually exclusive
157+
let result = cmd.try_get_matches_from(["test", "--write", "--show-hex"]);
158+
159+
assert!(result.is_err(), "Should fail because --show-hex requires --read but --write and --read are mutually exclusive");
160+
assert_eq!(result.unwrap_err().kind(), ErrorKind::MissingRequiredArgument);
161+
}

0 commit comments

Comments
 (0)