From 491c07f14f4a6dcb406f927263dc9f9d2469d80d Mon Sep 17 00:00:00 2001 From: Andy Francke Date: Mon, 13 Jul 2026 23:37:39 -0700 Subject: [PATCH] Reject shadowed global flags on proper subcommands Signed-off-by: Andy Francke --- cmd/sops/main.go | 33 +++++++++++++++ functional-tests/src/lib.rs | 80 +++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/cmd/sops/main.go b/cmd/sops/main.go index e590e1c82b..40482e157f 100644 --- a/cmd/sops/main.go +++ b/cmd/sops/main.go @@ -74,6 +74,21 @@ func warnMoreThanOnePositionalArgument(c *cli.Context) { } } +func rejectGlobalFlagsShadowedBySubcommand(c *cli.Context) error { + for _, flag := range c.Command.Flags { + for _, name := range strings.Split(flag.GetName(), ",") { + name = strings.TrimSpace(name) + if c.GlobalIsSet(name) && !c.IsSet(name) { + return common.NewExitError( + fmt.Sprintf("Error: --%s must be specified after the %s subcommand", name, c.Command.Name), + codes.ErrorConflictingParameters, + ) + } + } + } + return nil +} + func main() { cli.VersionPrinter = version.PrintVersion app := cli.NewApp() @@ -822,6 +837,9 @@ func main() { }, }, keyserviceFlags...), Action: func(c *cli.Context) error { + if err := rejectGlobalFlagsShadowedBySubcommand(c); err != nil { + return err + } if c.Bool("verbose") { logging.SetLevel(logrus.DebugLevel) } @@ -1008,6 +1026,9 @@ func main() { }, }, keyserviceFlags...), Action: func(c *cli.Context) error { + if err := rejectGlobalFlagsShadowedBySubcommand(c); err != nil { + return err + } if c.Bool("verbose") { logging.SetLevel(logrus.DebugLevel) } @@ -1194,6 +1215,9 @@ func main() { }, }, keyserviceFlags...), Action: func(c *cli.Context) error { + if err := rejectGlobalFlagsShadowedBySubcommand(c); err != nil { + return err + } if c.Bool("verbose") { logging.SetLevel(logrus.DebugLevel) } @@ -1368,6 +1392,9 @@ func main() { }, }, keyserviceFlags...), Action: func(c *cli.Context) error { + if err := rejectGlobalFlagsShadowedBySubcommand(c); err != nil { + return err + } if c.Bool("verbose") { logging.SetLevel(logrus.DebugLevel) } @@ -1482,6 +1509,9 @@ func main() { }, }, keyserviceFlags...), Action: func(c *cli.Context) error { + if err := rejectGlobalFlagsShadowedBySubcommand(c); err != nil { + return err + } if c.Bool("verbose") { logging.SetLevel(logrus.DebugLevel) } @@ -1610,6 +1640,9 @@ func main() { }, }, keyserviceFlags...), Action: func(c *cli.Context) error { + if err := rejectGlobalFlagsShadowedBySubcommand(c); err != nil { + return err + } if c.Bool("verbose") { logging.SetLevel(logrus.DebugLevel) } diff --git a/functional-tests/src/lib.rs b/functional-tests/src/lib.rs index d5c010aacb..e9df395778 100644 --- a/functional-tests/src/lib.rs +++ b/functional-tests/src/lib.rs @@ -1738,4 +1738,84 @@ bar: |- "Unexpected decrypted content" ); } + + #[test] + fn proper_subcommands_reject_shadowed_global_flags() { + let decrypt_flags: &[&[&str]] = &[ + &["--output-type", "json"], + &["--input-type", "json"], + &["--output", "ignored-output.json"], + &["--extract", "[\"foo\"]"], + &["--ignore-mac"], + ]; + + for flags in decrypt_flags { + let output = Command::new(SOPS_BINARY_PATH) + .args(*flags) + .arg("decrypt") + .arg("res/format.enc.yaml") + .output() + .expect("Error running sops"); + assert!( + !output.status.success(), + "sops unexpectedly accepted global flags: {flags:?}" + ); + assert!( + output.stdout.is_empty(), + "sops wrote output after rejecting global flags: {flags:?}" + ); + assert!( + String::from_utf8_lossy(&output.stderr) + .contains("must be specified after the decrypt subcommand"), + "sops did not explain the rejected global flags: {flags:?}" + ); + } + + for (subcommand, arguments) in [ + ("encrypt", vec!["nonexistent.yaml"]), + ("rotate", vec!["nonexistent.yaml"]), + ("edit", vec!["nonexistent.yaml"]), + ("set", vec!["nonexistent.yaml", "[\"foo\"]", "bar"]), + ("unset", vec!["nonexistent.yaml", "[\"foo\"]"]), + ] { + let output = Command::new(SOPS_BINARY_PATH) + .arg("--output-type") + .arg("json") + .arg(subcommand) + .args(arguments) + .output() + .expect("Error running sops"); + assert!( + !output.status.success(), + "sops unexpectedly accepted --output-type before {subcommand}" + ); + assert!( + output.stdout.is_empty(), + "sops wrote output after rejecting --output-type before {subcommand}" + ); + assert!( + String::from_utf8_lossy(&output.stderr).contains(&format!( + "must be specified after the {subcommand} subcommand" + )), + "sops did not explain the rejected --output-type before {subcommand}" + ); + } + } + + #[test] + fn decrypt_output_type_after_subcommand_emits_json() { + let output = Command::new(SOPS_BINARY_PATH) + .arg("decrypt") + .arg("--output-type") + .arg("json") + .arg("res/format.enc.yaml") + .output() + .expect("Error running sops"); + assert!(output.status.success(), "SOPS didn't return successfully"); + assert_eq!( + String::from_utf8_lossy(&output.stdout), + "{\n\t\"foo\": \"bar\"\n}\n", + "Unexpected decrypted content" + ); + } }