mirror of
https://github.com/codewhale-hq/Codewhale.git
synced 2026-11-09 01:49:49 -10:00
fix(config): finish credential and config validation repairs
Preserve the existing L1 repair work: restrict the generic Xiaomi MiMo
credential fallback to standard keys, refuse scalar replacement of profile
and workspace tables, and use the shared config vocabulary in the TUI loader.
Mask pasted credentials in project-provider and TUI validation errors while
keeping ordinary typo guidance readable.
Redact bare credentials inside Markdown and prose punctuation. Keep the
conservative opaque-key detector: lowercase keys containing separators are
still credentials, not automatically safe identifiers.
Verification on the repaired source/dependency tree:
- codewhale-secrets --lib: 55 passed, 0 failed, 1 ignored.
- codewhale-config --lib: 747 passed, 0 failed, 1 ignored.
- codewhale-tui --lib config_keys: 12 passed, 0 failed.
- npm test: 636 passed, 0 failed (68 wrapper, 16 SDK, 50 host, 502 web).
- npm run check:web: passed including production build, exit 0.
- scripts/check-provider-registry.py: passed.
- cargo fmt --all -- --check and git diff --check: passed.
Rust checks used two jobs, incremental compilation disabled, the existing
L1 target, and the machine build lock. The TUI linker reported its existing
large-unwind-table warning. The first web build rejected external dependency
symlinks; the successful retry used physical copies of matching-lockfile
dependencies without source changes.
Retains the independently verified provider-registry repair in 02b7bab70.
No hosted CI result or security-incident disposition is claimed here.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks
Signed-off-by: Hunter B <[email protected]>
This commit is contained in:
1 parent
02b7bab70d
commit
98a72b5edd
5 files changed
+219
-38
No files matched your search
@@ -3888,7 +3888,12 @@ pub fn load_project_config_outcome(workspace: &Path) -> ProjectConfigOutcome {
|
||||
return ProjectConfigOutcome::Invalid {
|
||||
path,
|
||||
reason: match raw_provider {
|
||||
Some(name) => format!("unknown provider '{name}'"),
|
||||
// A key pasted into `provider =` must not be
|
||||
// echoed; an ordinary typo stays readable.
|
||||
Some(name) => format!(
|
||||
"unknown provider '{}'",
|
||||
codewhale_secrets::redact::redact_secrets(&name)
|
||||
),
|
||||
None => "unknown provider".to_string(),
|
||||
},
|
||||
};
|
||||
@@ -4752,13 +4757,15 @@ fn xiaomi_mimo_base_url_uses_token_plan(base_url: &str) -> bool {
|
||||
|| normalized == XIAOMI_MIMO_TOKEN_PLAN_AMS_BASE_URL
|
||||
}
|
||||
|
||||
const XIAOMI_MIMO_TOKEN_PLAN_ENV_VARS: &[&str] =
|
||||
&["XIAOMI_MIMO_TOKEN_PLAN_API_KEY", "MIMO_TOKEN_PLAN_API_KEY"];
|
||||
const XIAOMI_MIMO_STANDARD_ENV_VARS: &[&str] =
|
||||
&["XIAOMI_MIMO_API_KEY", "XIAOMI_API_KEY", "MIMO_API_KEY"];
|
||||
|
||||
fn xiaomi_mimo_env_api_key_for_runtime(
|
||||
mode: Option<&str>,
|
||||
base_url: Option<&str>,
|
||||
) -> Option<String> {
|
||||
const TOKEN_PLAN_ENV_VARS: &[&str] =
|
||||
&["XIAOMI_MIMO_TOKEN_PLAN_API_KEY", "MIMO_TOKEN_PLAN_API_KEY"];
|
||||
const STANDARD_ENV_VARS: &[&str] = &["XIAOMI_MIMO_API_KEY", "XIAOMI_API_KEY", "MIMO_API_KEY"];
|
||||
let env_value = |vars: &[&str]| codewhale_secrets::env_first(vars).map(|(_, value)| value);
|
||||
|
||||
let normalized_mode =
|
||||
@@ -4768,7 +4775,7 @@ fn xiaomi_mimo_env_api_key_for_runtime(
|
||||
.is_some_and(xiaomi_mimo_mode_uses_standard_endpoint)
|
||||
|| base_url.is_some_and(xiaomi_mimo_base_url_is_pay_as_you_go);
|
||||
if standard_selected {
|
||||
return env_value(STANDARD_ENV_VARS);
|
||||
return env_value(XIAOMI_MIMO_STANDARD_ENV_VARS);
|
||||
}
|
||||
|
||||
let token_plan_selected = normalized_mode
|
||||
@@ -4777,10 +4784,10 @@ fn xiaomi_mimo_env_api_key_for_runtime(
|
||||
.is_some()
|
||||
|| base_url.is_some_and(xiaomi_mimo_base_url_uses_token_plan);
|
||||
if token_plan_selected {
|
||||
return env_value(TOKEN_PLAN_ENV_VARS);
|
||||
return env_value(XIAOMI_MIMO_TOKEN_PLAN_ENV_VARS);
|
||||
}
|
||||
|
||||
env_value(TOKEN_PLAN_ENV_VARS).or_else(|| env_value(STANDARD_ENV_VARS))
|
||||
env_value(XIAOMI_MIMO_TOKEN_PLAN_ENV_VARS).or_else(|| env_value(XIAOMI_MIMO_STANDARD_ENV_VARS))
|
||||
}
|
||||
|
||||
fn resolve_xiaomi_mimo_base_url(
|
||||
@@ -4995,8 +5002,19 @@ fn stored_api_key_for_provider(
|
||||
|
||||
/// The provider's API key from its own environment variables, the single
|
||||
/// list on its descriptor ([`provider::Provider::env_vars`]).
|
||||
///
|
||||
/// Xiaomi MiMo is the exception: its token-plan variables belong to the
|
||||
/// token-plan endpoints, and `xiaomi_mimo_env_api_key_for_runtime` (which the
|
||||
/// resolver tries first) is the only reader that knows the selected mode. This
|
||||
/// fallback reads the standard variables only, so a token-plan key is never
|
||||
/// sent to the pay-as-you-go endpoint.
|
||||
fn env_api_key_for_provider(provider: ProviderKind) -> Option<String> {
|
||||
codewhale_secrets::env_first(provider.provider().env_vars()).map(|(_, value)| value)
|
||||
let env_vars = if provider == ProviderKind::XiaomiMimo {
|
||||
XIAOMI_MIMO_STANDARD_ENV_VARS
|
||||
} else {
|
||||
provider.provider().env_vars()
|
||||
};
|
||||
codewhale_secrets::env_first(env_vars).map(|(_, value)| value)
|
||||
}
|
||||
|
||||
/// Whether an authentication mode requires API-key material.
|
||||
|
||||
@@ -3740,6 +3740,26 @@ base_url = "https://acme.example/v1"
|
||||
assert!(reason.contains("acme_zen_gateway"), "{reason}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn project_config_unknown_provider_reason_never_echoes_a_pasted_key() {
|
||||
let workspace = tempfile::tempdir().expect("workspace tempdir");
|
||||
let config_dir = workspace.path().join(CODEWHALE_APP_DIR);
|
||||
fs::create_dir_all(&config_dir).expect("mkdir project config");
|
||||
let key = ["sk-live-", "Z7qX4mNb2Vc9Lk3PwR8t"].concat();
|
||||
fs::write(
|
||||
config_dir.join(CONFIG_FILE_NAME),
|
||||
format!("provider = \"{key}\"\n"),
|
||||
)
|
||||
.expect("write project config");
|
||||
|
||||
let reason = load_project_config_outcome(workspace.path())
|
||||
.invalid()
|
||||
.map(|(_, reason)| reason.to_string())
|
||||
.expect("unknown provider is rejected");
|
||||
assert!(reason.starts_with("unknown provider"), "{reason}");
|
||||
assert!(!reason.contains(&key), "{reason}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn malformed_project_config_is_distinguishable_from_a_missing_one() {
|
||||
// #4733: the loader returned `None` for both cases. A project config can
|
||||
@@ -4242,7 +4262,13 @@ fn env_api_key_lookup_reads_exactly_each_providers_env_vars() {
|
||||
}
|
||||
unsafe { std::env::remove_var(var) };
|
||||
}
|
||||
let mut declared = provider.env_vars().to_vec();
|
||||
// Xiaomi MiMo's token-plan variables are read only by the mode-aware
|
||||
// lookup, never by this generic fallback.
|
||||
let mut declared = if kind == ProviderKind::XiaomiMimo {
|
||||
super::XIAOMI_MIMO_STANDARD_ENV_VARS.to_vec()
|
||||
} else {
|
||||
provider.env_vars().to_vec()
|
||||
};
|
||||
declared.sort_unstable();
|
||||
assert_eq!(reads, declared, "{}", kind.as_str());
|
||||
}
|
||||
@@ -7503,6 +7529,25 @@ fn xiaomi_mimo_env_pay_as_you_go_mode_prefers_standard_key() {
|
||||
assert_eq!(resolved.base_url, XIAOMI_MIMO_PAY_AS_YOU_GO_BASE_URL);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn xiaomi_mimo_pay_as_you_go_mode_never_falls_back_to_a_token_plan_key() {
|
||||
let _lock = env_lock();
|
||||
let _env = EnvGuard::without_deepseek_runtime_overrides();
|
||||
// Safety: test-only environment mutation guarded by a module mutex.
|
||||
unsafe {
|
||||
env::set_var("DEEPSEEK_PROVIDER", "xiaomi-mimo");
|
||||
env::set_var("XIAOMI_MIMO_MODE", "pay-as-you-go");
|
||||
env::set_var("MIMO_TOKEN_PLAN_API_KEY", "tp-env-key");
|
||||
}
|
||||
|
||||
let resolved = ConfigToml::default().resolve_runtime_options(&CliRuntimeOverrides::default());
|
||||
|
||||
assert_eq!(resolved.provider, ProviderKind::XiaomiMimo);
|
||||
assert_eq!(resolved.base_url, XIAOMI_MIMO_PAY_AS_YOU_GO_BASE_URL);
|
||||
assert_eq!(resolved.api_key, None);
|
||||
assert_eq!(resolved.api_key_source, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn novita_env_overrides_key_and_model_when_config_missing() {
|
||||
let _lock = env_lock();
|
||||
|
||||
@@ -244,9 +244,9 @@ fn redact_line(line: &str, policy: RedactionPolicy) -> String {
|
||||
changed = true;
|
||||
masked.push(word.replace(trimmed, &redacted));
|
||||
spaced = SpacedAssignment::None;
|
||||
} else if !trimmed.is_empty() && looks_like_secret_token(trimmed) {
|
||||
} else if let Some(core) = secret_token_core(trimmed) {
|
||||
changed = true;
|
||||
masked.push(word.replace(trimmed, REDACTED));
|
||||
masked.push(word.replacen(core, REDACTED, 1));
|
||||
spaced = SpacedAssignment::None;
|
||||
} else if let Some(redacted) = redact_structured_word(trimmed, policy) {
|
||||
changed = true;
|
||||
@@ -700,6 +700,23 @@ const BARE_OPAQUE_TOKEN_PREFIXES: &[&str] = &["xai-", "nvapi-", "gsk_", "hf_", "
|
||||
/// [`SECRET_TOKEN_PREFIXES`] word, a provider-prefixed opaque key, an AWS
|
||||
/// access key id, or a JWT.
|
||||
fn looks_like_secret_token(word: &str) -> bool {
|
||||
secret_token_core(word).is_some()
|
||||
}
|
||||
|
||||
/// The credential inside `word` once markdown and prose punctuation around it
|
||||
/// is dropped (`` `xai-…` ``, `(nvapi-…)`, `AKIA…:`, a JWT ending a
|
||||
/// sentence), or `None` when the word is not a credential on its own.
|
||||
fn secret_token_core(word: &str) -> Option<&str> {
|
||||
let core = word.trim_matches(|c| {
|
||||
matches!(
|
||||
c,
|
||||
'(' | ')' | '[' | ']' | '{' | '}' | '<' | '>' | ':' | '`' | '.' | '!' | '?'
|
||||
)
|
||||
});
|
||||
(!core.is_empty() && is_bare_secret_token(core)).then_some(core)
|
||||
}
|
||||
|
||||
fn is_bare_secret_token(word: &str) -> bool {
|
||||
SECRET_TOKEN_PREFIXES
|
||||
.iter()
|
||||
.any(|p| word.len() > p.len() + 6 && word.starts_with(p))
|
||||
@@ -793,6 +810,44 @@ mod bare_token_tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_bare_token_wrapped_in_markdown_or_prose_punctuation_is_masked() {
|
||||
let body = "Z7qX4mNb2Vc9Lk3PwR8t";
|
||||
let aws = ["AKIA", "Z7QX4MNB2VC9LK3P"].concat();
|
||||
let jwt = ["eyJhbGciOiJIUzI1NiJ9", ".eyJzdWIiOiIxIn0", ".c2lnbmF0dXJl"].concat();
|
||||
for (wrapped, token) in [
|
||||
(format!("`xai-{body}`"), format!("xai-{body}")),
|
||||
(format!("(nvapi-{body})"), format!("nvapi-{body}")),
|
||||
(format!("hf_{body})"), format!("hf_{body}")),
|
||||
(format!("[pplx-{body}]"), format!("pplx-{body}")),
|
||||
(format!("{aws}:"), aws.clone()),
|
||||
(format!("{aws})"), aws.clone()),
|
||||
(format!("{jwt})."), jwt.clone()),
|
||||
] {
|
||||
let out = redact_secrets(&format!("key {wrapped} rejected"));
|
||||
assert!(!out.contains(&token), "{out}");
|
||||
assert!(out.contains(REDACTED), "{out}");
|
||||
}
|
||||
assert_eq!(
|
||||
redact_secrets(&format!("key `xai-{body}` rejected")),
|
||||
"key `[redacted]` rejected"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lowercase_opaque_keys_with_separators_are_still_masked() {
|
||||
// Lowercase letters and separators are also valid random key material;
|
||||
// their shape alone does not establish that this is an identifier.
|
||||
for prefix in BARE_OPAQUE_TOKEN_PREFIXES {
|
||||
for separator in ['-', '_', '.'] {
|
||||
let body = ["a1b2c3d4", "e5f6g7h8", "i9j0k1l2"].join(&separator.to_string());
|
||||
let token = format!("{prefix}{body}");
|
||||
let out = redact_secrets(&format!("request failed: `{token}` rejected"));
|
||||
assert_eq!(out, "request failed: `[redacted]` rejected");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn identifiers_sharing_a_prefix_stay_visible() {
|
||||
for word in [
|
||||
|
||||
+20
-26
@@ -5196,32 +5196,26 @@ impl Config {
|
||||
provider.as_str()
|
||||
);
|
||||
}
|
||||
if let Some(policy) = self.approval_policy.as_deref() {
|
||||
let normalized = policy.trim().to_ascii_lowercase();
|
||||
if !matches!(
|
||||
normalized.as_str(),
|
||||
"on-request" | "untrusted" | "never" | "auto" | "suggest"
|
||||
) {
|
||||
anyhow::bail!(
|
||||
"Invalid approval_policy '{policy}': expected on-request, untrusted, never, auto, or suggest."
|
||||
);
|
||||
}
|
||||
}
|
||||
if let Some(v) = self.verbosity.as_deref() {
|
||||
let normalized = v.trim().to_ascii_lowercase();
|
||||
if !matches!(normalized.as_str(), "normal" | "concise") {
|
||||
anyhow::bail!("Invalid verbosity '{v}': expected normal or concise.");
|
||||
}
|
||||
}
|
||||
if let Some(mode) = self.sandbox_mode.as_deref() {
|
||||
let normalized = mode.trim().to_ascii_lowercase();
|
||||
if !matches!(
|
||||
normalized.as_str(),
|
||||
"read-only" | "workspace-write" | "danger-full-access" | "external-sandbox"
|
||||
) {
|
||||
anyhow::bail!(
|
||||
"Invalid sandbox_mode '{mode}': expected read-only, workspace-write, danger-full-access, or external-sandbox."
|
||||
);
|
||||
// One vocabulary with `codewhale config set`, which refuses the same
|
||||
// values before writing them (`codewhale_config::config_toml_choices`).
|
||||
for (key, value) in [
|
||||
("approval_policy", self.approval_policy.as_deref()),
|
||||
("verbosity", self.verbosity.as_deref()),
|
||||
("sandbox_mode", self.sandbox_mode.as_deref()),
|
||||
] {
|
||||
let (Some(value), Some(choices)) = (value, codewhale_config::config_toml_choices(key))
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
if !choices.contains(&value.trim().to_ascii_lowercase().as_str()) {
|
||||
let expected = match choices.split_last() {
|
||||
Some((last, [])) => (*last).to_string(),
|
||||
Some((last, [only])) => format!("{only} or {last}"),
|
||||
Some((last, rest)) => format!("{}, or {last}", rest.join(", ")),
|
||||
None => String::new(),
|
||||
};
|
||||
let displayed_value = codewhale_secrets::redact::redact_secrets(value);
|
||||
anyhow::bail!("Invalid {key} '{displayed_value}': expected {expected}.");
|
||||
}
|
||||
}
|
||||
if let Some(tui) = &self.tui
|
||||
|
||||
@@ -43,6 +43,12 @@ const OTHER_READER_ROOT_KEYS: &[&str] = &[
|
||||
"tui.stream_chunk_timeout_secs",
|
||||
];
|
||||
|
||||
/// [`OTHER_READER_ROOT_KEYS`] whose value is a table of user entries
|
||||
/// (`[profiles.<name>]`, `[projects.'<path>']`, `[workspace.'<path>']`).
|
||||
/// `config doctor` counts them as read, but `config set <key> <value>` would
|
||||
/// replace the whole table with a string, so a scalar write is refused.
|
||||
const TABLE_ROOT_KEYS: &[&str] = &["profiles", "projects", "workspace"];
|
||||
|
||||
/// Where a key a user asked `config set` to write is read from.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum ConfigKeyHome {
|
||||
@@ -156,7 +162,7 @@ fn settable_keys() -> impl Iterator<Item = &'static str> {
|
||||
OTHER_READER_ROOT_KEYS
|
||||
.iter()
|
||||
.copied()
|
||||
.filter(|key| !key.contains('.')),
|
||||
.filter(|key| !key.contains('.') && !TABLE_ROOT_KEYS.contains(key)),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -230,6 +236,12 @@ pub(crate) fn settings_toml_keys() -> impl Iterator<Item = &'static str> {
|
||||
/// [`ReasoningEffort::parse_strict`]: crate::reasoning_preference::ReasoningEffort::parse_strict
|
||||
pub fn config_toml_value(key: &str, value: &str) -> Result<Option<toml::Value>> {
|
||||
let key = key.trim();
|
||||
if TABLE_ROOT_KEYS.contains(&key) {
|
||||
anyhow::bail!(
|
||||
"`{key}` is a table of entries, so `config set {key}` would replace all of \
|
||||
them; nothing was saved. Edit the [{key}.<name>] table in config.toml instead."
|
||||
);
|
||||
}
|
||||
codewhale_config::check_config_toml_choice(key, value)?;
|
||||
if key == "reasoning_effort" {
|
||||
let effort = crate::reasoning_preference::ReasoningEffort::parse_strict(value)
|
||||
@@ -474,6 +486,63 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn scalar_set_of_a_table_root_key_is_refused() {
|
||||
// `config set workspace /path` used to store `workspace = "/path"`,
|
||||
// replacing every `[workspace.'<path>']` entry on save.
|
||||
for key in TABLE_ROOT_KEYS {
|
||||
assert_eq!(config_key_home(key), ConfigKeyHome::ConfigToml, "{key}");
|
||||
let error = config_toml_value(key, "/some/path").expect_err(key);
|
||||
assert!(error.to_string().contains("nothing was saved"), "{error}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_set_and_loader_accept_the_same_closed_vocabulary() {
|
||||
for (key, value, valid) in [
|
||||
("approval_policy", " ON-REQUEST ", true),
|
||||
("approval_policy", "ask", false),
|
||||
("verbosity", " CONCISE ", true),
|
||||
("verbosity", "quiet", false),
|
||||
("sandbox_mode", " WORKSPACE-WRITE ", true),
|
||||
("sandbox_mode", "full", false),
|
||||
] {
|
||||
let mut config = crate::config::Config::default();
|
||||
match key {
|
||||
"approval_policy" => config.approval_policy = Some(value.to_string()),
|
||||
"verbosity" => config.verbosity = Some(value.to_string()),
|
||||
"sandbox_mode" => config.sandbox_mode = Some(value.to_string()),
|
||||
_ => unreachable!(),
|
||||
}
|
||||
assert_eq!(config_toml_value(key, value).is_ok(), valid, "{key}");
|
||||
assert_eq!(config.validate().is_ok(), valid, "{key}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loader_choice_errors_redact_pasted_keys_but_keep_ordinary_typos() {
|
||||
let token = ["sk-live-", "Z7qX4mNb2Vc9Lk3PwR8t"].concat();
|
||||
for key in ["approval_policy", "verbosity", "sandbox_mode"] {
|
||||
for value in [token.as_str(), "misspelled-choice"] {
|
||||
let mut config = crate::config::Config::default();
|
||||
match key {
|
||||
"approval_policy" => config.approval_policy = Some(value.to_string()),
|
||||
"verbosity" => config.verbosity = Some(value.to_string()),
|
||||
"sandbox_mode" => config.sandbox_mode = Some(value.to_string()),
|
||||
_ => unreachable!(),
|
||||
}
|
||||
let error = config.validate().expect_err("invalid choice").to_string();
|
||||
assert!(!error.contains(&token));
|
||||
assert!(error.contains("expected"));
|
||||
assert!(error.contains(if value == token {
|
||||
"[redacted]"
|
||||
} else {
|
||||
"misspelled-choice"
|
||||
}));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn documented_user_workspace_entry_is_read() {
|
||||
// The block docs/CONFIGURATION.md gives under "User workspace entries".
|
||||
|
||||
Reference in new issue
Block a user