From 70241a292e8070d0bee234e1bbdd10ebd315c42e Mon Sep 17 00:00:00 2001 From: Autumn Skerritt Date: Thu, 1 Oct 2026 17:04:48 +0000 Subject: [PATCH] fix(config): let command-line options override the config file `Opts::merge` copied every value present in the config file over the parsed command line, so e.g. `-r 8600-8650` was silently replaced by the config's `range` and the requested ports were never scanned (`-p` only survived because `PortStrategy::pick` prefers `ports` over `range`). The same happened to `-a`, `-b`, `-t`, `--tries`, `-g`, `--scripts`, `--scan-order`, the trailing nmap command and every other merged option. This contradicts the documented behaviour: "Any arguments passed directly to the binary will override the default provided in this config file". `Opts::parse()` cannot tell a value typed by the user from a clap default, so `Opts::read()` now parses through `ArgMatches` and records the ids of the arguments whose `value_source` is `CommandLine` in a new skipped `cli_args` field. `merge_required`/`merge_optional` leave those fields alone; options not given on the command line are still filled in from the config file. `--top` and `--no-config` behave as before. Add pure parsing tests (no sockets) covering both directions. Fixes #722 --- src/input.rs | 135 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 129 insertions(+), 6 deletions(-) diff --git a/src/input.rs b/src/input.rs index ff8cae6..e7d9488 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1,6 +1,9 @@ //! Provides a means to read, parse and hold configuration options for scans. -use clap::{Parser, ValueEnum}; +use clap::parser::ValueSource; +use clap::{CommandFactory, FromArgMatches, Parser, ValueEnum}; use serde_derive::Deserialize; +use std::collections::HashSet; +use std::ffi::OsString; use std::fs; use std::path::PathBuf; @@ -231,18 +234,48 @@ pub struct Opts { /// scanning the next one, for slow, low-noise scans. 0 disables the delay. #[arg(long, default_value = "0", value_name = "MS")] pub interval: u64, + + /// Ids of the options given explicitly on the command line. The + /// configuration file never overrides these (see [`Opts::merge`]). + #[arg(skip)] + pub cli_args: HashSet, } #[cfg(not(tarpaulin_include))] impl Opts { pub fn read() -> Self { - let mut opts = Opts::parse(); + Self::try_read_from(std::env::args_os()).unwrap_or_else(|e| e.exit()) + } + + /// Parses `args` like [`Opts::read`] does, returning an error instead of + /// exiting. + fn try_read_from(args: I) -> Result + where + I: IntoIterator, + T: Into + Clone, + { + let mut command = Self::command(); + let mut matches = command.try_get_matches_from_mut(args)?; + + // clap fills in default values, so the parsed struct alone cannot + // tell `-b 4500` apart from no `-b` at all. Record what the user + // actually typed so that `merge` lets it win over the config file. + let cli_args = command + .get_arguments() + .map(|arg| arg.get_id().as_str()) + .filter(|id| matches.value_source(id) == Some(ValueSource::CommandLine)) + .map(str::to_owned) + .collect(); + + let mut opts = + Self::from_arg_matches_mut(&mut matches).map_err(|e| e.format(&mut command))?; + opts.cli_args = cli_args; if opts.ports.is_none() && opts.range.is_none() { opts.range = Some(PortRanges(vec![(LOWEST_PORT_NUMBER, TOP_PORT_NUMBER)])); } - opts + Ok(opts) } /// Validates options whose availability or semantics depend on the @@ -267,7 +300,8 @@ impl Opts { } /// Reads the command line arguments into an Opts struct and merge - /// values found within the user configuration file. + /// values found within the user configuration file. Options given on the + /// command line take precedence over the configuration file. pub fn merge(&mut self, config: &Config) { if !self.no_config { self.merge_required(config); @@ -280,7 +314,9 @@ impl Opts { ($($field: ident),+) => { $( if let Some(e) = &config.$field { - self.$field = e.clone(); + if !self.cli_args.contains(stringify!($field)) { + self.$field = e.clone(); + } } )+ } @@ -296,7 +332,7 @@ impl Opts { macro_rules! merge_optional { ($($field: ident),+) => { $( - if config.$field.is_some() { + if config.$field.is_some() && !self.cli_args.contains(stringify!($field)) { self.$field = config.$field.clone(); } )+ @@ -337,6 +373,7 @@ impl Default for Opts { udp: false, closed: false, interval: 0, + cli_args: HashSet::new(), } } } @@ -459,6 +496,7 @@ pub fn old_default_config_path() -> PathBuf { mod tests { use clap::{CommandFactory, Parser}; use parameterized::parameterized; + use std::collections::HashSet; use super::{Config, Opts, PortRanges, ScanOrder, ScriptsRequired}; @@ -635,6 +673,91 @@ mod tests { assert_eq!(opts.resolver, config.resolver); } + #[test] + fn cli_arguments_override_config() { + // Regression test for #722: the config file used to silently replace + // options given on the command line, e.g. `-r` with its `range`. + let mut opts = Opts::try_read_from([ + "rustscan", + "-a", + "192.168.0.1", + "-r", + "8600-8650", + "-b", + "100", + // Passing clap's default value explicitly must still win. + "-t", + "1500", + "--tries", + "3", + "--scan-order", + "serial", + "--scripts", + "none", + "-g", + "--", + "-sV", + ]) + .unwrap(); + let config: Config = toml::from_str( + r#" + addresses = ["127.0.0.1"] + range = { start = 1, end = 100 } + batch_size = 25000 + timeout = 1000 + tries = 1 + scan_order = "Random" + scripts = "Custom" + greppable = false + command = ["-A"] + "#, + ) + .unwrap(); + + opts.merge(&config); + + assert_eq!(opts.addresses, vec!["192.168.0.1".to_owned()]); + assert_eq!(opts.range, Some(PortRanges(vec![(8_600, 8_650)]))); + assert_eq!(opts.batch_size, 100); + assert_eq!(opts.timeout, 1_500); + assert_eq!(opts.tries, 3); + assert_eq!(opts.scan_order, ScanOrder::Serial); + assert_eq!(opts.scripts, ScriptsRequired::None); + assert!(opts.greppable); + assert_eq!(opts.command, vec!["-sV".to_owned()]); + } + + #[test] + fn config_fills_arguments_not_given_on_cli() { + let mut opts = Opts::try_read_from(["rustscan", "-a", "192.168.0.1"]).unwrap(); + let config: Config = toml::from_str( + r#" + addresses = ["127.0.0.1"] + range = { start = 1, end = 100 } + batch_size = 25000 + timeout = 1000 + tries = 3 + scan_order = "Random" + greppable = true + command = ["-A"] + "#, + ) + .unwrap(); + + opts.merge(&config); + + // Only `-a` was typed; clap defaults such as `-b 4500` must not count. + assert_eq!(opts.cli_args, HashSet::from(["addresses".to_owned()])); + assert_eq!(opts.addresses, vec!["192.168.0.1".to_owned()]); + assert_eq!(opts.range, Some(PortRanges(vec![(1, 100)]))); + assert_eq!(opts.batch_size, 25_000); + assert_eq!(opts.timeout, 1_000); + assert_eq!(opts.tries, 3); + assert_eq!(opts.scan_order, ScanOrder::Random); + assert!(opts.greppable); + assert_eq!(opts.command, vec!["-A".to_owned()]); + } + #[test] fn parses_comma_separated_ranges() { let opts = Opts::parse_from([