Repository: Orange-OpenSource/hurl
File: packages/hurl/src/cli/options/matches.rs (Rust)

Code before:

pub fn resolves(arg_matches: &ArgMatches) -> Vec<String> {
    get_strings(arg_matches, "resolve").unwrap_or_default()
}

pub fn retry(arg_matches: &ArgMatches) -> Option<Count> {
    match get::<i32>(arg_matches, "retry") {
        Some(-1) => Some(Count::Infinite),
        Some(r) => Some(Count::Finite(r as usize)),
        None => None,
    }
}

pub fn retry_interval(arg_matches: &ArgMatches) -> Result<Duration, CliOptionsError> {
    let s = get::<String>(arg_matches, "retry_interval").unwrap_or("1000".to_string());
    get_duration(&s, DurationUnit::MilliSecond)
}

pub fn secret(matches: &ArgMatches) -> Result<HashMap<String, String>, CliOptionsError> {
    let mut secrets = HashMap::new();

    if let Some(filenames) = get_strings(matches, "secrets_file") {
        for f in &filenames {
            let filename = Path::new(f);
            let vars = VariablesFile::open(filename, TypeKind::String)?;
            for var in vars {
                let (name, value) = var?;
                if let Value::String(value) = value {
                    secrets.insert(name.to_string(), value);
                }
            }
        }
    }

    if let Some(secret) = get_strings(matches, "secret") {
        for s in secret {
            let inferred = false;
            let (name, value) = variables::parse(&s, inferred)?;
            // We check that there is no existing secrets
            if secrets.contains_key(&name) {
                return Err(CliOptionsError::Error(format!(
                    "secret '{}' can't be reassigned",
                    &name
                )));
            }
            // Secrets can only be string.
            if let Value::String(value) = value {
                secrets.insert(name, value);
            }
        }
    }

Review comment (security):
We should add this check

```rust
let (name, value) = var?;
if secrets.contains_key(&name) {
    return Err(CliOptionsError::Error(format!(
        "secret '{}' can't be reassigned",
        &name
    )));
}
//...
```
There's a redundancy with the later code but for the moment this is not a problem

Code after:

pub fn resolves(arg_matches: &ArgMatches) -> Vec<String> {
    get_strings(arg_matches, "resolve").unwrap_or_default()
}

pub fn retry(arg_matches: &ArgMatches) -> Option<Count> {
    match get::<i32>(arg_matches, "retry") {
        Some(-1) => Some(Count::Infinite),
        Some(r) => Some(Count::Finite(r as usize)),
        None => None,
    }
}

pub fn retry_interval(arg_matches: &ArgMatches) -> Result<Duration, CliOptionsError> {
    let s = get::<String>(arg_matches, "retry_interval").unwrap_or("1000".to_string());
    get_duration(&s, DurationUnit::MilliSecond)
}

pub fn secret(matches: &ArgMatches) -> Result<HashMap<String, String>, CliOptionsError> {
    let mut secrets = HashMap::new();

    if let Some(filenames) = get_strings(matches, "secrets_file") {
        for f in &filenames {
            let filename = Path::new(f);
            let vars = VariablesFile::open(filename, TypeKind::String)?;
            for var in vars {
                let (name, value) = var?;
                // We check that there is no existing secrets
                if secrets.contains_key(&name) {
                    return Err(CliOptionsError::Error(format!(
                        "secret '{}' can't be reassigned",
                        &name
                    )));
                }
                // Secrets can only be string.
                if let Value::String(value) = value {
                    secrets.insert(name.to_string(), value);
                }
            }
        }
    }

    if let Some(secret) = get_strings(matches, "secret") {
        for s in secret {
            let inferred = false;
            let (name, value) = variables::parse(&s, inferred)?;
            if secrets.contains_key(&name) {
                return Err(CliOptionsError::Error(format!(
                    "secret '{}' can't be reassigned",
                    &name
                )));