From d73e5786be135740273c79c0392611a8d77b156e Mon Sep 17 00:00:00 2001 From: Joel Natividad <1980690+jqnatividad@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:12:12 -0400 Subject: [PATCH 1/2] fix(describegpt): redact --api-key and --ckan-token from the attribution describegpt's attribution records the command line that ran, and it is copied into every generated dictionary, description and tag set -- files meant to be shared. The values of -k/--api-key and --ckan-token are now replaced with [REDACTED] in every spelling docopt accepts: `--opt value`, `--opt=value`, and stuck or clustered short options (`-kKEY`, `-Ak KEY`). qsv's docopt rejects abbreviated long options, so exact names suffice. Arguments after `--` are left alone. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/cmd/describegpt.rs | 168 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 165 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a1d65b9c0..545399144a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Building qsv from source now also patches `hashbrown` to Polars' fork (the same pin Polars itself uses), because Polars 2.0 relies on prefetch APIs that aren't in a hashbrown release yet ([rust-lang/hashbrown#727](https://github.com/rust-lang/hashbrown/pull/727)). ### Fixed +- **`describegpt` no longer writes your API key into the files it generates.** The attribution it adds to each data dictionary, description and tag set records the command line that ran, and that included the value of `--api-key` (or `-k`) and `--ckan-token`. Those values are now replaced with `[REDACTED]`, however they were written (`--api-key KEY`, `--api-key=KEY`, `-kKEY`, `-Ak KEY`). Files generated by earlier versions still contain the key, so redact or regenerate any you have shared, and consider rotating the key. Passing it in `QSV_LLM_APIKEY` (or `QSV_CKAN_TOKEN`) instead keeps it out of the command line, and out of `ps`, altogether. - **`viz --geojson auto` (and `census:*`) handles an explicit `--feature-id-key` consistently on every path** ([#4735](https://github.com/dathere/qsv/issues/4735), [#4763](https://github.com/dathere/qsv/pull/4763)). [#4734](https://github.com/dathere/qsv/pull/4734) fixed this for state columns; three other paths still used the key auto picked: - **County names and geocoded place names** resolve to Census GEOIDs, so they can only match on `properties.GEOID`. With another key (e.g. `properties.NAME`) they passed the coverage check and then drew a map that dropped every row. They now refuse the run and say to omit `--feature-id-key`. This applies to `viz choropleth` and `viz smart`. - **`viz smart` tries the next region column** when the explicit key is missing from one column's boundaries (e.g. `properties.STUSAB` on counties). Before, the whole run failed. diff --git a/src/cmd/describegpt.rs b/src/cmd/describegpt.rs index 5369ba3391..61c197ee46 100644 --- a/src/cmd/describegpt.rs +++ b/src/cmd/describegpt.rs @@ -2257,13 +2257,27 @@ fn extract_json_from_output(output: &str) -> CliResult { ) } +/// Stands in for a secret option value in the provenance command line. +const REDACTED_ARG: &str = "[REDACTED]"; + fn provenance_command_line() -> String { normalize_command_line(std::env::args()) } /// The pure half of [`provenance_command_line`], split out so it is testable without mutating the /// process's own argv. +/// +/// The values of secret-bearing options (`-k`/`--api-key`, `--ckan-token`) are replaced with +/// [`REDACTED_ARG`]: the command line is copied into every generated artifact, which is meant to be +/// shared. qsv's docopt rejects abbreviated long options, but accepts `--opt=value` and stuck or +/// clustered short options (`-kKEY`, `-Ak KEY`), so all of those spellings are covered. fn normalize_command_line>(mut args: I) -> String { + const SECRET_LONG_OPTS: [&str; 2] = ["--api-key", "--ckan-token"]; + const SECRET_SHORT_OPT: char = 'k'; + // describegpt's short options that take a value; in a cluster, the rest of the token (or the + // next argument) is that value + const VALUE_SHORT_OPTS: &str = "kmoptu"; + let Some(argv0) = args.next() else { return String::new(); }; @@ -2272,10 +2286,54 @@ fn normalize_command_line>(mut args: I) -> String { let program = std::path::Path::new(&argv0) .file_name() .map_or_else(|| argv0.clone(), |n| n.to_string_lossy().into_owned()); - std::iter::once(program) - .chain(args) - .collect::>() - .join(" ") + + let mut out = vec![program]; + let mut redact_next = false; + let mut positional_only = false; + for arg in args { + if redact_next { + out.push(REDACTED_ARG.to_string()); + redact_next = false; + } else if positional_only || arg == "-" || !arg.starts_with('-') { + out.push(arg); + } else if arg == "--" { + positional_only = true; + out.push(arg); + } else if arg.starts_with("--") { + let (name, value) = arg + .split_once('=') + .map_or((arg.as_str(), None), |(n, v)| (n, Some(v))); + if SECRET_LONG_OPTS.contains(&name) { + if value.is_some() { + out.push(format!("{name}={REDACTED_ARG}")); + } else { + out.push(arg); + redact_next = true; + } + } else { + out.push(arg); + } + } else { + // a short option cluster, e.g. `-q`, `-kKEY`, `-Ak KEY` or `-m` `model` + let mut redacted = None; + for (i, c) in arg.char_indices().skip(1) { + if !VALUE_SHORT_OPTS.contains(c) { + continue; + } + if c == SECRET_SHORT_OPT { + let end = i + c.len_utf8(); + if end == arg.len() { + redact_next = true; + } else { + redacted = Some(format!("{}{REDACTED_ARG}", &arg[..end])); + } + } + break; + } + out.push(redacted.unwrap_or(arg)); + } + } + out.join(" ") } /// Replace {`GENERATED_BY_SIGNATURE`} placeholder with actual attribution @@ -7757,6 +7815,108 @@ mod tests { assert_eq!(line(&["..", "x"]), ".. x"); } + /// The provenance command line is copied into every generated (shareable) artifact, so the + /// values of secret-bearing options must never appear in it, however they were spelled. + #[test] + fn normalize_command_line_redacts_secret_option_values() { + let line = |v: &[&str]| normalize_command_line(v.iter().map(ToString::to_string)); + let key = "sk-or-v1-0123456789abcdef"; + + for (argv, expected) in [ + ( + vec!["qsv", "describegpt", "in.csv", "--api-key", key, "--all"], + "qsv describegpt in.csv --api-key [REDACTED] --all", + ), + ( + vec![ + "qsv", + "describegpt", + "in.csv", + "--api-key=sk-or-v1-0123456789abcdef", + ], + "qsv describegpt in.csv --api-key=[REDACTED]", + ), + ( + vec!["qsv", "describegpt", "-k", key, "in.csv"], + "qsv describegpt -k [REDACTED] in.csv", + ), + ( + vec![ + "qsv", + "describegpt", + "-ksk-or-v1-0123456789abcdef", + "in.csv", + ], + "qsv describegpt -k[REDACTED] in.csv", + ), + // `-A` is a flag, so `-Ak` is `--all` followed by `-k` + ( + vec!["qsv", "describegpt", "-Ak", key, "in.csv"], + "qsv describegpt -Ak [REDACTED] in.csv", + ), + ( + vec![ + "qsv", + "describegpt", + "-qAksk-or-v1-0123456789abcdef", + "in.csv", + ], + "qsv describegpt -qAk[REDACTED] in.csv", + ), + ( + vec![ + "qsv", + "describegpt", + "--ckan-token", + "tok-123", + "--ckan-token=tok-456", + ], + "qsv describegpt --ckan-token [REDACTED] --ckan-token=[REDACTED]", + ), + ] { + let got = line(&argv); + assert_eq!(got, expected, "{argv:?}"); + assert!( + !got.contains("sk-or-v1") && !got.contains("tok-"), + "{argv:?}" + ); + } + + // not secrets: values of other options, including ones containing a `k`, are verbatim + for argv in [ + // `-m` takes a value, so the `k` in `-mkimi` is part of the model name + vec![ + "qsv", + "describegpt", + "-mkimi-k2", + "-u", + "http://localhost:1234/v1", + ], + vec!["qsv", "describegpt", "-p", "rank by kWh", "--model", "k"], + // only exact option names: `--ckan-api` is not `--ckan-token` + vec![ + "qsv", + "describegpt", + "--ckan-api", + "https://ckan.example/api/3/action", + ], + vec!["qsv", "describegpt", "in.csv", "-"], + ] { + assert_eq!(line(&argv), argv.join(" "), "{argv:?}"); + } + + // after `--`, everything is a positional argument + assert_eq!( + line(&["qsv", "describegpt", "--", "-k", "--api-key"]), + "qsv describegpt -- -k --api-key" + ); + // a trailing secret option with no value: nothing to redact, and no panic + assert_eq!( + line(&["qsv", "describegpt", "--api-key"]), + "qsv describegpt --api-key" + ); + } + #[test] fn parse_language_option_classifies_thresholds_and_explicit_languages() { // unset -> autodetect with NO confidence floor (agreement is the gate) From 53f110b6c186693d691bcee8697b18ac11a2e6de Mon Sep 17 00:00:00 2001 From: Joel Natividad <1980690+jqnatividad@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:12:20 -0400 Subject: [PATCH 2/2] docs(changelog): cite #4766 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 545399144a..636cb60a80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Building qsv from source now also patches `hashbrown` to Polars' fork (the same pin Polars itself uses), because Polars 2.0 relies on prefetch APIs that aren't in a hashbrown release yet ([rust-lang/hashbrown#727](https://github.com/rust-lang/hashbrown/pull/727)). ### Fixed -- **`describegpt` no longer writes your API key into the files it generates.** The attribution it adds to each data dictionary, description and tag set records the command line that ran, and that included the value of `--api-key` (or `-k`) and `--ckan-token`. Those values are now replaced with `[REDACTED]`, however they were written (`--api-key KEY`, `--api-key=KEY`, `-kKEY`, `-Ak KEY`). Files generated by earlier versions still contain the key, so redact or regenerate any you have shared, and consider rotating the key. Passing it in `QSV_LLM_APIKEY` (or `QSV_CKAN_TOKEN`) instead keeps it out of the command line, and out of `ps`, altogether. +- **`describegpt` no longer writes your API key into the files it generates** ([#4766](https://github.com/dathere/qsv/pull/4766)). The attribution it adds to each data dictionary, description and tag set records the command line that ran, and that included the value of `--api-key` (or `-k`) and `--ckan-token`. Those values are now replaced with `[REDACTED]`, however they were written (`--api-key KEY`, `--api-key=KEY`, `-kKEY`, `-Ak KEY`). Files generated by earlier versions still contain the key, so redact or regenerate any you have shared, and consider rotating the key. Passing it in `QSV_LLM_APIKEY` (or `QSV_CKAN_TOKEN`) instead keeps it out of the command line, and out of `ps`, altogether. - **`viz --geojson auto` (and `census:*`) handles an explicit `--feature-id-key` consistently on every path** ([#4735](https://github.com/dathere/qsv/issues/4735), [#4763](https://github.com/dathere/qsv/pull/4763)). [#4734](https://github.com/dathere/qsv/pull/4734) fixed this for state columns; three other paths still used the key auto picked: - **County names and geocoded place names** resolve to Census GEOIDs, so they can only match on `properties.GEOID`. With another key (e.g. `properties.NAME`) they passed the coverage check and then drew a map that dropped every row. They now refuse the run and say to omit `--feature-id-key`. This applies to `viz choropleth` and `viz smart`. - **`viz smart` tries the next region column** when the explicit key is missing from one column's boundaries (e.g. `properties.STUSAB` on counties). Before, the whole run failed.