fix: save config with restrictive permissions and improve secret redaction (#833)
* fix: save config with restrictive permissions and improve secret redaction - Config files containing API keys were written with default permissions (typically 0644), making them world-readable on multi-user systems. Use OpenOptions with mode 0o600 on Unix to restrict access to the file owner. - `redact_secret` threshold raised from 8 to 16 characters — previously a 9-character secret would leak 8 of its 9 characters (4 prefix + 4 suffix). Now secrets up to 16 chars are fully masked with "********". * fix(config): keep secret saves warning-free on windows --------- Co-authored-by: Hunter Bown <hmbown@gmail.com>
This commit is contained in:
@@ -1,5 +1,7 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::fs;
|
||||
#[cfg(unix)]
|
||||
use std::io::Write;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::OnceLock;
|
||||
|
||||
@@ -7,6 +9,9 @@ use anyhow::{Context, Result, bail};
|
||||
pub use deepseek_secrets::Secrets;
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
#[cfg(unix)]
|
||||
use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
|
||||
|
||||
pub const CONFIG_FILE_NAME: &str = "config.toml";
|
||||
const DEFAULT_DEEPSEEK_MODEL: &str = "deepseek-v4-pro";
|
||||
const DEFAULT_NVIDIA_NIM_MODEL: &str = "deepseek-ai/deepseek-v4-pro";
|
||||
@@ -933,8 +938,30 @@ impl ConfigStore {
|
||||
})?;
|
||||
}
|
||||
let body = toml::to_string_pretty(&self.config).context("failed to serialize config")?;
|
||||
fs::write(&self.path, body)
|
||||
.with_context(|| format!("failed to write config at {}", self.path.display()))?;
|
||||
#[cfg(unix)]
|
||||
{
|
||||
let mut file = fs::OpenOptions::new()
|
||||
.write(true)
|
||||
.create(true)
|
||||
.truncate(true)
|
||||
.mode(0o600)
|
||||
.open(&self.path)
|
||||
.with_context(|| format!("failed to write config at {}", self.path.display()))?;
|
||||
file.write_all(body.as_bytes())
|
||||
.with_context(|| format!("failed to write config at {}", self.path.display()))?;
|
||||
file.set_permissions(fs::Permissions::from_mode(0o600))
|
||||
.with_context(|| {
|
||||
format!(
|
||||
"failed to set config permissions at {}",
|
||||
self.path.display()
|
||||
)
|
||||
})?;
|
||||
}
|
||||
#[cfg(not(unix))]
|
||||
{
|
||||
fs::write(&self.path, body)
|
||||
.with_context(|| format!("failed to write config at {}", self.path.display()))?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -995,7 +1022,7 @@ fn parse_bool(raw: &str) -> Result<bool> {
|
||||
}
|
||||
|
||||
fn redact_secret(secret: &str) -> String {
|
||||
if secret.len() <= 8 {
|
||||
if secret.len() <= 16 {
|
||||
return "********".to_string();
|
||||
}
|
||||
format!("{}***{}", &secret[..4], &secret[secret.len() - 4..])
|
||||
@@ -1361,6 +1388,51 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_values_fully_redacts_short_api_key() {
|
||||
let config = ConfigToml {
|
||||
api_key: Some("short-key".to_string()),
|
||||
..ConfigToml::default()
|
||||
};
|
||||
|
||||
let values = config.list_values();
|
||||
|
||||
assert_eq!(values.get("api_key").map(String::as_str), Some("********"));
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn save_clamps_existing_config_permissions() {
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
|
||||
let unique = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.expect("clock")
|
||||
.as_nanos();
|
||||
let dir = std::env::temp_dir().join(format!(
|
||||
"deepseek-config-perms-{}-{unique}",
|
||||
std::process::id()
|
||||
));
|
||||
fs::create_dir_all(&dir).expect("mkdir");
|
||||
let path = dir.join(CONFIG_FILE_NAME);
|
||||
fs::write(&path, "api_key = \"old\"\n").expect("seed config");
|
||||
fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).expect("chmod seed");
|
||||
|
||||
let store = ConfigStore {
|
||||
path: path.clone(),
|
||||
config: ConfigToml {
|
||||
api_key: Some("new-secret".to_string()),
|
||||
..ConfigToml::default()
|
||||
},
|
||||
};
|
||||
store.save().expect("save");
|
||||
|
||||
let mode = fs::metadata(&path).expect("metadata").permissions().mode() & 0o777;
|
||||
assert_eq!(mode, 0o600);
|
||||
|
||||
let _ = fs::remove_dir_all(dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_kind_parses_openrouter_and_novita_aliases() {
|
||||
assert_eq!(
|
||||
|
||||
Reference in New Issue
Block a user