fix: write to real file if config.yaml is symlink (#7669)
Signed-off-by: lsytj0413 <511121939@qq.com>
This commit is contained in:
@@ -459,20 +459,57 @@ impl Config {
|
|||||||
load_init_config_from_workspace()
|
load_init_config_from_workspace()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn config_write_target_path(&self) -> Result<PathBuf, ConfigError> {
|
||||||
|
let mut path = self.config_path.clone();
|
||||||
|
|
||||||
|
// Follow symlinks so we update the target file without replacing the link itself.
|
||||||
|
const MAX_SYMLINK_HOPS: usize = 1;
|
||||||
|
let mut hops = 0usize;
|
||||||
|
loop {
|
||||||
|
match std::fs::symlink_metadata(&path) {
|
||||||
|
Ok(meta) if meta.file_type().is_symlink() => {
|
||||||
|
if hops >= MAX_SYMLINK_HOPS {
|
||||||
|
return Err(std::io::Error::new(
|
||||||
|
std::io::ErrorKind::InvalidInput,
|
||||||
|
format!(
|
||||||
|
"Too many symlink levels (or a cycle) while resolving config path: {:?}",
|
||||||
|
self.config_path
|
||||||
|
),
|
||||||
|
)
|
||||||
|
.into());
|
||||||
|
}
|
||||||
|
hops += 1;
|
||||||
|
|
||||||
|
let link = std::fs::read_link(&path)?;
|
||||||
|
path = if link.is_absolute() {
|
||||||
|
link
|
||||||
|
} else {
|
||||||
|
path.parent().unwrap_or_else(|| Path::new(".")).join(link)
|
||||||
|
};
|
||||||
|
}
|
||||||
|
Ok(_) => return Ok(path),
|
||||||
|
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(path),
|
||||||
|
Err(e) => return Err(e.into()),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn save_values(&self, values: &Mapping) -> Result<(), ConfigError> {
|
fn save_values(&self, values: &Mapping) -> Result<(), ConfigError> {
|
||||||
// Create backup before writing new config
|
// Create backup before writing new config
|
||||||
self.create_backup_if_needed()?;
|
self.create_backup_if_needed()?;
|
||||||
|
|
||||||
|
let target_path = self.config_write_target_path()?;
|
||||||
|
|
||||||
// Convert to YAML for storage
|
// Convert to YAML for storage
|
||||||
let yaml_value = serde_yaml::to_string(values)?;
|
let yaml_value = serde_yaml::to_string(values)?;
|
||||||
|
|
||||||
if let Some(parent) = self.config_path.parent() {
|
if let Some(parent) = target_path.parent() {
|
||||||
std::fs::create_dir_all(parent)
|
std::fs::create_dir_all(parent)
|
||||||
.map_err(|e| ConfigError::DirectoryError(e.to_string()))?;
|
.map_err(|e| ConfigError::DirectoryError(e.to_string()))?;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Write to a temporary file first for atomic operation
|
// Write to a temporary file first for atomic operation
|
||||||
let temp_path = self.config_path.with_extension("tmp");
|
let temp_path = target_path.with_extension("tmp");
|
||||||
|
|
||||||
{
|
{
|
||||||
let mut file = OpenOptions::new()
|
let mut file = OpenOptions::new()
|
||||||
@@ -493,7 +530,7 @@ impl Config {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Atomically replace the original file
|
// Atomically replace the original file
|
||||||
std::fs::rename(&temp_path, &self.config_path)?;
|
std::fs::rename(&temp_path, &target_path)?;
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
@@ -1029,7 +1066,7 @@ pub fn load_init_config_from_workspace() -> Result<Mapping, ConfigError> {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use serial_test::serial;
|
use serial_test::serial;
|
||||||
use tempfile::NamedTempFile;
|
use tempfile::{NamedTempFile, TempDir};
|
||||||
#[test]
|
#[test]
|
||||||
fn test_basic_config() -> Result<(), ConfigError> {
|
fn test_basic_config() -> Result<(), ConfigError> {
|
||||||
let config = new_test_config();
|
let config = new_test_config();
|
||||||
@@ -1250,6 +1287,78 @@ mod tests {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
#[cfg(unix)]
|
||||||
|
fn test_write_follows_symlink() -> Result<(), ConfigError> {
|
||||||
|
use std::os::unix::fs as unix_fs;
|
||||||
|
|
||||||
|
let dir = TempDir::new().unwrap();
|
||||||
|
let target_path = dir.path().join("real_config.yaml");
|
||||||
|
let symlink_path = dir.path().join("config.yaml");
|
||||||
|
|
||||||
|
std::fs::write(&target_path, "{}\n")?;
|
||||||
|
unix_fs::symlink(&target_path, &symlink_path)?;
|
||||||
|
|
||||||
|
let secrets_file = NamedTempFile::new().unwrap();
|
||||||
|
let config = Config::new_with_file_secrets(&symlink_path, secrets_file.path())?;
|
||||||
|
|
||||||
|
config.set_param("key1", "value1")?;
|
||||||
|
|
||||||
|
let meta = std::fs::symlink_metadata(&symlink_path)?;
|
||||||
|
assert!(
|
||||||
|
meta.file_type().is_symlink(),
|
||||||
|
"config path should remain a symlink"
|
||||||
|
);
|
||||||
|
|
||||||
|
let content = std::fs::read_to_string(&symlink_path)?;
|
||||||
|
assert!(content.contains("key1: value1"));
|
||||||
|
|
||||||
|
let content = std::fs::read_to_string(&target_path)?;
|
||||||
|
assert!(content.contains("key1: value1"));
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
#[cfg(unix)]
|
||||||
|
fn test_write_fails_on_long_symlink_chain() -> Result<(), ConfigError> {
|
||||||
|
use std::os::unix::fs as unix_fs;
|
||||||
|
|
||||||
|
let dir = TempDir::new().unwrap();
|
||||||
|
let target_path = dir.path().join("real_config.yaml");
|
||||||
|
std::fs::write(&target_path, "{}\n")?;
|
||||||
|
|
||||||
|
// config.yaml -> link1.yaml -> real_config.yaml
|
||||||
|
// We only allow following one symlink hop. If there's another symlink, we should fail
|
||||||
|
// rather than overwrite the intermediate symlink.
|
||||||
|
let config_symlink = dir.path().join("config.yaml");
|
||||||
|
let link1 = dir.path().join("link1.yaml");
|
||||||
|
unix_fs::symlink(&target_path, &link1)?;
|
||||||
|
unix_fs::symlink(&link1, &config_symlink)?;
|
||||||
|
|
||||||
|
let secrets_file = NamedTempFile::new().unwrap();
|
||||||
|
let config = Config::new_with_file_secrets(&config_symlink, secrets_file.path())?;
|
||||||
|
|
||||||
|
let err = config.set_param("key1", "value1").unwrap_err();
|
||||||
|
assert!(
|
||||||
|
err.to_string().contains("Too many symlink levels"),
|
||||||
|
"unexpected error: {err}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let meta = std::fs::symlink_metadata(&config_symlink)?;
|
||||||
|
assert!(
|
||||||
|
meta.file_type().is_symlink(),
|
||||||
|
"config path should remain a symlink"
|
||||||
|
);
|
||||||
|
let meta = std::fs::symlink_metadata(&link1)?;
|
||||||
|
assert!(
|
||||||
|
meta.file_type().is_symlink(),
|
||||||
|
"intermediate link should remain a symlink"
|
||||||
|
);
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_config_recovery_from_backup() -> Result<(), ConfigError> {
|
fn test_config_recovery_from_backup() -> Result<(), ConfigError> {
|
||||||
let config_file = NamedTempFile::new().unwrap();
|
let config_file = NamedTempFile::new().unwrap();
|
||||||
|
|||||||
Reference in New Issue
Block a user