Using SecretString to store senstive values in copy stmt

[MichaelScofield]

What problem does the new feature solve?

copy stmt can carry senstive data like S3 API key or secret. This makes leaking easily.

What does the feature do?

Right now the senstive values are all stored in plain strings, and we have to write special redacting codes for printing them. It's better we use SecretString for them.

A follow up of #3744.

Implementation challenges

[tisonkun]

Current code -

• feat: redact secrets in sql when logging #2141
• fix: improve redact sql regexp #3080

A thoroughly solution should be, supporting reconstruct query from statements, and redact options of copy stmt on demand.

See:

match self.query_statement(stmt, query_ctx.clone()).await {
    Ok(output) => {
        let output_result =
            query_interceptor.post_execute(output, query_ctx.clone());
        results.push(output_result);
    }
    Err(e) => {
        let redacted = sql::util::redact_sql_secrets(query.as_ref());
        error!(e; "Failed to execute query: {redacted}");

        results.push(Err(e));
        break;
    }
}

We currently print all the query if any of the statement failed to execute.

[tisonkun]

I found this issue not quite easy as it may be like.

We hold all the options in WITH and CONNECTION as:

    pub with: OptionMap,
    pub connection: OptionMap,

where:

pub struct OptionMap {
    pub map: HashMap<String, String>,
}

Either we change map: HashMap<String, String> to map: HashMap<String, SecretString>, or we have two inner map, they doesn't sound a good idea.

We now have a function to structurally Display these options:

fn redact_and_sort_options(options: &OptionMap) -> Vec<String> {
    let options = options.as_ref();
    let mut result = Vec::with_capacity(options.len());
    let keys = options.keys().sorted();
    for key in keys {
        if let Some(val) = options.get(key) {
            let redacted = REDACTED_OPTIONS
                .iter()
                .any(|opt| opt.eq_ignore_ascii_case(key));
            if redacted {
                result.push(format!("{key} = '******'"));
            } else {
                result.push(format!("{key} = '{}'", val.escape_default()));
            }
        }
    }
    result
}

We may even implement Display for OptionMap with this functionality, so it's also reliable and can structurally redact the options.

[MichaelScofield]

We can make the sensetive data stored in their own fields, rather then the OptionMap.

[tisonkun]

Then the code to access an option value is somehow awkward. If we insist in trying SecretString, I may suggest:

pub struct OptionMap {
    map: HashMap<String, String>,
    secrets: HashMap<String, SecretString >,
}

And do the merge read within the encapsulation of OptionMap.

[MichaelScofield]

I'm ok with moving all the senstive data to a "secrets" map like that.