fix(macos): harden OAuth credential directory recovery
- Normalize auth paths before comparison and credential scanning - Update effective v8 auth-dir fields without requiring a version marker - Preserve active credentials and configuration when recovery fails - Add regression tests for config reloads and v7-to-v8 upgrades
This commit is contained in:
1 parent
cb7fbbecfc
commit
6dd8646afc
3 files changed
+321
-38
No files matched your search
@@ -521,12 +521,7 @@ pub(crate) fn apply_gui_managed_settings(
|
||||
serde_norway::to_value(config.port)
|
||||
.map_err(|err| format!("Failed to serialize kernel port: {err}"))?,
|
||||
)?;
|
||||
changed |= set_core_yaml_schema_value(
|
||||
document,
|
||||
&["auth-dir"],
|
||||
&["oauth", "auth-dir"],
|
||||
serde_norway::Value::String(config.auth_dir.clone()),
|
||||
)?;
|
||||
changed |= set_core_yaml_auth_dir(document, &config.auth_dir)?;
|
||||
changed |= set_core_yaml_schema_value(
|
||||
document,
|
||||
&["debug"],
|
||||
@@ -1100,10 +1095,10 @@ fn auth_directory_has_json_files(path: &Path) -> Result<bool, String> {
|
||||
for entry in entries {
|
||||
let entry =
|
||||
entry.map_err(|error| format!("Failed to inspect OAuth directory entry: {error}"))?;
|
||||
if entry
|
||||
let file_type = entry
|
||||
.file_type()
|
||||
.map_err(|error| format!("Failed to inspect OAuth file type: {error}"))?
|
||||
.is_file()
|
||||
.map_err(|error| format!("Failed to inspect OAuth file type: {error}"))?;
|
||||
if (file_type.is_file() || file_type.is_symlink())
|
||||
&& entry
|
||||
.path()
|
||||
.extension()
|
||||
@@ -1121,12 +1116,13 @@ pub(crate) fn recover_macos_relative_oauth_dir(
|
||||
install_dir: &Path,
|
||||
persistent_auth_dir: &Path,
|
||||
) -> Result<bool, String> {
|
||||
if Path::new(config.auth_dir.trim()) != Path::new(OAUTH_DIR_NAME) {
|
||||
let current_auth_dir =
|
||||
normalize_path_lexically(&auth_dir_path_for_core(&config.auth_dir, install_dir));
|
||||
if current_auth_dir != normalize_path_lexically(&install_dir.join(OAUTH_DIR_NAME)) {
|
||||
return Ok(false);
|
||||
}
|
||||
let current_auth_dir = auth_dir_path_for_core(&config.auth_dir, install_dir);
|
||||
if !auth_directory_has_json_files(persistent_auth_dir)?
|
||||
|| auth_directory_has_json_files(¤t_auth_dir)?
|
||||
if auth_directory_has_json_files(¤t_auth_dir)?
|
||||
|| !auth_directory_has_json_files(persistent_auth_dir)?
|
||||
{
|
||||
return Ok(false);
|
||||
}
|
||||
@@ -1136,17 +1132,25 @@ pub(crate) fn recover_macos_relative_oauth_dir(
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
fn migrate_packaged_macos_auth_dir(config: &mut GuiConfigFile) -> Result<bool, String> {
|
||||
let previous_auth_dir = config.auth_dir.clone();
|
||||
let install_dir = core_install_dir()?;
|
||||
let persistent_auth_dir = fixed_oauth_dir()?;
|
||||
migrate_macos_auth_dir_at(config, &install_dir, &persistent_auth_dir)
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "macos", test))]
|
||||
pub(crate) fn migrate_macos_auth_dir_at(
|
||||
config: &mut GuiConfigFile,
|
||||
install_dir: &Path,
|
||||
persistent_auth_dir: &Path,
|
||||
) -> Result<bool, String> {
|
||||
let mut candidate = config.clone();
|
||||
let migrated =
|
||||
migrate_auth_dir_from_macos_app_bundle(config, &install_dir, &persistent_auth_dir)?;
|
||||
let recovered = recover_macos_relative_oauth_dir(config, &install_dir, &persistent_auth_dir)?;
|
||||
migrate_auth_dir_from_macos_app_bundle(&mut candidate, install_dir, persistent_auth_dir)?;
|
||||
let recovered = recover_macos_relative_oauth_dir(&mut candidate, install_dir, persistent_auth_dir)?;
|
||||
if migrated || recovered {
|
||||
if let Err(error) = patch_core_auth_dir(&config.auth_dir) {
|
||||
config.auth_dir = previous_auth_dir;
|
||||
return Err(format!("Failed to update kernel OAuth directory: {error}"));
|
||||
}
|
||||
patch_core_auth_dir_at(&install_dir.join(CORE_CONFIG_FILE), &candidate.auth_dir)
|
||||
.map_err(|error| format!("Failed to update kernel OAuth directory: {error}"))?;
|
||||
*config = candidate;
|
||||
}
|
||||
Ok(migrated || recovered)
|
||||
}
|
||||
@@ -1679,7 +1683,12 @@ pub(crate) fn sanitize_gui_config(config: &mut GuiConfigFile) -> Result<bool, St
|
||||
}
|
||||
#[cfg(target_os = "macos")]
|
||||
{
|
||||
changed |= migrate_packaged_macos_auth_dir(config)?;
|
||||
match migrate_packaged_macos_auth_dir(config) {
|
||||
Ok(migrated) => changed |= migrated,
|
||||
Err(error) => {
|
||||
eprintln!("Failed to recover macOS OAuth directory; keeping existing configuration: {error}");
|
||||
}
|
||||
}
|
||||
}
|
||||
let legacy_default_auth_dir = fixed_oauth_dir()?;
|
||||
if config.auth_dir.trim().is_empty()
|
||||
|
||||
@@ -431,19 +431,39 @@ pub(crate) fn patch_core_management_secret_key(secret_key: &str) -> Result<(), S
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
pub(crate) fn patch_core_auth_dir(auth_dir: &str) -> Result<(), String> {
|
||||
let auth_dir = auth_dir.to_string();
|
||||
patch_existing_core_config(move |document| {
|
||||
let v8 = core_config_uses_v8(document);
|
||||
set_core_yaml_path_value(
|
||||
document,
|
||||
if v8 { &["oauth", "auth-dir"] } else { &["auth-dir"] },
|
||||
serde_norway::Value::String(auth_dir),
|
||||
)
|
||||
#[cfg(any(target_os = "macos", test))]
|
||||
pub(crate) fn patch_core_auth_dir_at(config_path: &Path, auth_dir: &str) -> Result<(), String> {
|
||||
patch_existing_core_config_at(config_path, |document| {
|
||||
set_core_yaml_auth_dir(document, auth_dir)
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn set_core_yaml_auth_dir(
|
||||
document: &mut serde_norway::Value,
|
||||
auth_dir: &str,
|
||||
) -> Result<bool, String> {
|
||||
let root = document
|
||||
.as_mapping()
|
||||
.ok_or_else(|| "Kernel configuration root must be a YAML mapping".to_string())?;
|
||||
let nested = core_config_uses_v8(document)
|
||||
|| nested_yaml_value(root, &["oauth", "auth-dir"]).is_some();
|
||||
let has_legacy = yaml_mapping_value(root, "auth-dir").is_some();
|
||||
let value = serde_norway::Value::String(auth_dir.to_string());
|
||||
let mut changed = set_core_yaml_path_value(
|
||||
document,
|
||||
if nested {
|
||||
&["oauth", "auth-dir"]
|
||||
} else {
|
||||
&["auth-dir"]
|
||||
},
|
||||
value.clone(),
|
||||
)?;
|
||||
if nested && has_legacy {
|
||||
changed |= set_core_yaml_path_value(document, &["auth-dir"], value)?;
|
||||
}
|
||||
Ok(changed)
|
||||
}
|
||||
|
||||
pub(crate) fn patch_core_plugins_enabled(enabled: bool) -> Result<(), String> {
|
||||
patch_existing_core_config(|document| {
|
||||
set_core_yaml_nested_value(
|
||||
@@ -567,19 +587,30 @@ pub(crate) fn patch_existing_core_config<F>(update: F) -> Result<(), String>
|
||||
where
|
||||
F: FnOnce(&mut serde_norway::Value) -> Result<bool, String>,
|
||||
{
|
||||
let _config_guard = lock_core_config_file()?;
|
||||
let config_path = core_install_dir()?.join(CORE_CONFIG_FILE);
|
||||
if !config_path.is_file() {
|
||||
return Ok(());
|
||||
}
|
||||
patch_existing_core_config_at(&config_path, update)
|
||||
}
|
||||
|
||||
let content = fs::read_to_string(&config_path)
|
||||
.map_err(|err| format!("Failed to read kernel configuration {}: {err}", path_to_string(&config_path)))?;
|
||||
fn patch_existing_core_config_at<F>(config_path: &Path, update: F) -> Result<(), String>
|
||||
where
|
||||
F: FnOnce(&mut serde_norway::Value) -> Result<bool, String>,
|
||||
{
|
||||
let _config_guard = lock_core_config_file()?;
|
||||
let content = match fs::read_to_string(config_path) {
|
||||
Ok(content) => content,
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()),
|
||||
Err(error) => {
|
||||
return Err(format!(
|
||||
"Failed to read kernel configuration {}: {error}",
|
||||
path_to_string(config_path)
|
||||
));
|
||||
}
|
||||
};
|
||||
let Some(updated) = patch_core_yaml_document(&content, update)? else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
write_yaml_if_changed(&config_path, &updated)?;
|
||||
write_yaml_if_changed(config_path, &updated)?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -240,6 +240,249 @@ fn macos_relative_oauth_dir_recovers_existing_persistent_credentials() {
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn macos_oauth_recovery_accepts_equivalent_directory_paths() {
|
||||
let root = agent_test_home("macos-oauth-equivalent-paths");
|
||||
let install_dir = root.join("cpa-core");
|
||||
let persistent_auth_dir = root.join(OAUTH_DIR_NAME);
|
||||
fs::create_dir_all(&persistent_auth_dir).unwrap();
|
||||
fs::write(persistent_auth_dir.join("account.json"), b"credential").unwrap();
|
||||
for auth_dir in [
|
||||
"./oauth".to_string(),
|
||||
"oauth/./".to_string(),
|
||||
"../cpa-core/oauth".to_string(),
|
||||
path_to_string(&install_dir.join(OAUTH_DIR_NAME)),
|
||||
] {
|
||||
let mut config = GuiConfigFile {
|
||||
auth_dir: auth_dir.clone(),
|
||||
..GuiConfigFile::default()
|
||||
};
|
||||
assert!(
|
||||
recover_macos_relative_oauth_dir(&mut config, &install_dir, &persistent_auth_dir).unwrap(),
|
||||
"failed to recover {auth_dir}"
|
||||
);
|
||||
assert_eq!(config.auth_dir, DEFAULT_AUTH_DIR);
|
||||
}
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn macos_oauth_recovery_preserves_credentials_behind_parent_components() {
|
||||
let root = agent_test_home("macos-oauth-parent-components");
|
||||
let install_dir = root.join("cpa-core");
|
||||
let active_auth_dir = install_dir.join(OAUTH_DIR_NAME);
|
||||
let persistent_auth_dir = root.join(OAUTH_DIR_NAME);
|
||||
fs::create_dir_all(&active_auth_dir).unwrap();
|
||||
fs::create_dir_all(&persistent_auth_dir).unwrap();
|
||||
fs::write(active_auth_dir.join("current.json"), b"current-credential").unwrap();
|
||||
fs::write(persistent_auth_dir.join("old.json"), b"old-credential").unwrap();
|
||||
assert!(!install_dir.join("missing").exists());
|
||||
|
||||
for auth_dir in [
|
||||
"missing/../oauth".to_string(),
|
||||
path_to_string(&install_dir.join("missing").join("..").join(OAUTH_DIR_NAME)),
|
||||
] {
|
||||
#[cfg(unix)]
|
||||
assert_eq!(
|
||||
fs::read_dir(auth_dir_path_for_core(&auth_dir, &install_dir))
|
||||
.unwrap_err()
|
||||
.kind(),
|
||||
io::ErrorKind::NotFound
|
||||
);
|
||||
let mut config = GuiConfigFile {
|
||||
auth_dir: auth_dir.clone(),
|
||||
..GuiConfigFile::default()
|
||||
};
|
||||
assert!(
|
||||
!migrate_macos_auth_dir_at(&mut config, &install_dir, &persistent_auth_dir).unwrap()
|
||||
);
|
||||
assert_eq!(config.auth_dir, auth_dir);
|
||||
}
|
||||
assert_eq!(
|
||||
fs::read(active_auth_dir.join("current.json")).unwrap(),
|
||||
b"current-credential"
|
||||
);
|
||||
assert_eq!(
|
||||
fs::read(persistent_auth_dir.join("old.json")).unwrap(),
|
||||
b"old-credential"
|
||||
);
|
||||
assert!(!install_dir.join("missing").exists());
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn oauth_directory_write_updates_the_effective_v8_field_without_version_marker() {
|
||||
for input in [
|
||||
"oauth: {auth-dir: oauth}\n",
|
||||
"auth-dir: ../oauth\noauth: {auth-dir: oauth}\n",
|
||||
] {
|
||||
let config = GuiConfigFile::default();
|
||||
let updated = apply_gui_managed_settings(input, &config).unwrap();
|
||||
let document = serde_norway::from_str(&updated).unwrap();
|
||||
assert_eq!(
|
||||
core_config_settings_from_value(&document).unwrap().auth_dir,
|
||||
DEFAULT_AUTH_DIR
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn macos_oauth_recovery_survives_config_reload_and_v8_upgrade() {
|
||||
for input in [
|
||||
"auth-dir: oauth\n",
|
||||
"config-version: 8\noauth:\n auth-dir: oauth\n",
|
||||
"oauth:\n auth-dir: ./oauth\n",
|
||||
"auth-dir: ../oauth\noauth: {auth-dir: oauth}\n",
|
||||
] {
|
||||
let root = agent_test_home("macos-oauth-recovery-reload");
|
||||
let install_dir = root.join("cpa-core");
|
||||
let persistent_auth_dir = root.join(OAUTH_DIR_NAME);
|
||||
let core_config_path = install_dir.join(CORE_CONFIG_FILE);
|
||||
let gui_config_path = root.join(GUI_CONFIG_FILE);
|
||||
let credential_path = persistent_auth_dir.join("account.json");
|
||||
fs::create_dir_all(&install_dir).unwrap();
|
||||
fs::create_dir_all(&persistent_auth_dir).unwrap();
|
||||
fs::write(&credential_path, b"existing-credential").unwrap();
|
||||
fs::write(&core_config_path, format!("# user comment\n{input}")).unwrap();
|
||||
let original_document = serde_norway::from_str(input).unwrap();
|
||||
let mut config = GuiConfigFile {
|
||||
auth_dir: core_config_settings_from_value(&original_document)
|
||||
.unwrap()
|
||||
.auth_dir,
|
||||
..GuiConfigFile::default()
|
||||
};
|
||||
ensure_strong_management_secret(&mut config).unwrap();
|
||||
write_gui_config_to_path(&config, &gui_config_path).unwrap();
|
||||
|
||||
assert!(
|
||||
migrate_macos_auth_dir_at(&mut config, &install_dir, &persistent_auth_dir).unwrap()
|
||||
);
|
||||
let patched = fs::read_to_string(&core_config_path).unwrap();
|
||||
assert!(patched.contains("# user comment"));
|
||||
let patched_document: serde_norway::Value = serde_norway::from_str(&patched).unwrap();
|
||||
assert_eq!(
|
||||
core_config_settings_from_value(&patched_document).unwrap().auth_dir,
|
||||
DEFAULT_AUTH_DIR
|
||||
);
|
||||
if let Some(legacy) = patched_document.get("auth-dir") {
|
||||
assert_eq!(legacy.as_str(), Some(DEFAULT_AUTH_DIR));
|
||||
}
|
||||
|
||||
write_gui_config_to_path(&config, &gui_config_path).unwrap();
|
||||
let mut reloaded: GuiConfigFile =
|
||||
toml::from_str(&fs::read_to_string(&gui_config_path).unwrap()).unwrap();
|
||||
assert_eq!(reloaded.auth_dir, DEFAULT_AUTH_DIR);
|
||||
assert!(
|
||||
!migrate_macos_auth_dir_at(&mut reloaded, &install_dir, &persistent_auth_dir).unwrap()
|
||||
);
|
||||
assert_eq!(fs::read_to_string(&core_config_path).unwrap(), patched);
|
||||
|
||||
let staging_dir = root.join("next-core");
|
||||
fs::create_dir_all(&staging_dir).unwrap();
|
||||
fs::write(
|
||||
staging_dir.join(CORE_EXAMPLE_CONFIG_FILE),
|
||||
"config-version: 8\noauth:\n auth-dir: oauth\n",
|
||||
)
|
||||
.unwrap();
|
||||
migrate_core_config_for_update(&install_dir, &staging_dir).unwrap();
|
||||
let upgraded = fs::read_to_string(staging_dir.join(CORE_CONFIG_FILE)).unwrap();
|
||||
let started = apply_gui_managed_settings(&upgraded, &reloaded).unwrap();
|
||||
let started_document = serde_norway::from_str(&started).unwrap();
|
||||
let effective = core_config_settings_from_value(&started_document).unwrap();
|
||||
assert_eq!(effective.auth_dir, DEFAULT_AUTH_DIR);
|
||||
assert_eq!(
|
||||
auth_dir_path_for_core(&effective.auth_dir, &install_dir),
|
||||
persistent_auth_dir
|
||||
);
|
||||
assert_eq!(fs::read(&credential_path).unwrap(), b"existing-credential");
|
||||
assert!(!install_dir.join(OAUTH_DIR_NAME).join("account.json").exists());
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn macos_oauth_recovery_preserves_config_when_inspection_or_patch_fails() {
|
||||
for failure in ["invalid-yaml", "unreadable-config", "invalid-auth-directory"] {
|
||||
let root = agent_test_home("macos-oauth-recovery-failure");
|
||||
let install_dir = root.join("cpa-core");
|
||||
let persistent_auth_dir = root.join(OAUTH_DIR_NAME);
|
||||
let core_config_path = install_dir.join(CORE_CONFIG_FILE);
|
||||
fs::create_dir_all(&install_dir).unwrap();
|
||||
fs::create_dir_all(&persistent_auth_dir).unwrap();
|
||||
fs::write(persistent_auth_dir.join("account.json"), b"existing-credential").unwrap();
|
||||
match failure {
|
||||
"invalid-yaml" => fs::write(&core_config_path, "oauth: [\n").unwrap(),
|
||||
"unreadable-config" => fs::create_dir(&core_config_path).unwrap(),
|
||||
_ => {
|
||||
fs::write(&core_config_path, "auth-dir: oauth\n").unwrap();
|
||||
fs::write(install_dir.join(OAUTH_DIR_NAME), b"not a directory").unwrap();
|
||||
}
|
||||
}
|
||||
let mut config = GuiConfigFile {
|
||||
auth_dir: OAUTH_DIR_NAME.to_string(),
|
||||
port: 9123,
|
||||
..GuiConfigFile::default()
|
||||
};
|
||||
let original_config = toml::to_string(&config).unwrap();
|
||||
let original_yaml = fs::read(&core_config_path).ok();
|
||||
|
||||
assert!(migrate_macos_auth_dir_at(&mut config, &install_dir, &persistent_auth_dir).is_err());
|
||||
assert_eq!(toml::to_string(&config).unwrap(), original_config);
|
||||
assert_eq!(fs::read(&core_config_path).ok(), original_yaml);
|
||||
assert_eq!(
|
||||
fs::read(persistent_auth_dir.join("account.json")).unwrap(),
|
||||
b"existing-credential"
|
||||
);
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn macos_oauth_recovery_requires_existing_persistent_credentials() {
|
||||
let root = agent_test_home("macos-oauth-recovery-empty");
|
||||
let install_dir = root.join("cpa-core");
|
||||
let persistent_auth_dir = root.join(OAUTH_DIR_NAME);
|
||||
let mut config = GuiConfigFile {
|
||||
auth_dir: OAUTH_DIR_NAME.to_string(),
|
||||
..GuiConfigFile::default()
|
||||
};
|
||||
assert!(
|
||||
!migrate_macos_auth_dir_at(&mut config, &install_dir, &persistent_auth_dir).unwrap()
|
||||
);
|
||||
fs::create_dir_all(persistent_auth_dir.join("nested")).unwrap();
|
||||
fs::write(persistent_auth_dir.join("core.log"), b"log").unwrap();
|
||||
fs::write(persistent_auth_dir.join("nested").join("account.json"), b"nested").unwrap();
|
||||
assert!(
|
||||
!migrate_macos_auth_dir_at(&mut config, &install_dir, &persistent_auth_dir).unwrap()
|
||||
);
|
||||
assert_eq!(config.auth_dir, OAUTH_DIR_NAME);
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn macos_oauth_recovery_preserves_active_linked_credentials() {
|
||||
let root = agent_test_home("macos-oauth-recovery-symlink");
|
||||
let install_dir = root.join("cpa-core");
|
||||
let active_auth_dir = install_dir.join(OAUTH_DIR_NAME);
|
||||
let persistent_auth_dir = root.join(OAUTH_DIR_NAME);
|
||||
fs::create_dir_all(&active_auth_dir).unwrap();
|
||||
fs::create_dir_all(&persistent_auth_dir).unwrap();
|
||||
let credential_path = persistent_auth_dir.join("account.json");
|
||||
fs::write(&credential_path, b"existing-credential").unwrap();
|
||||
std::os::unix::fs::symlink(&credential_path, active_auth_dir.join("linked.json")).unwrap();
|
||||
let mut config = GuiConfigFile {
|
||||
auth_dir: OAUTH_DIR_NAME.to_string(),
|
||||
..GuiConfigFile::default()
|
||||
};
|
||||
assert!(
|
||||
!migrate_macos_auth_dir_at(&mut config, &install_dir, &persistent_auth_dir).unwrap()
|
||||
);
|
||||
assert_eq!(config.auth_dir, OAUTH_DIR_NAME);
|
||||
assert_eq!(fs::read(active_auth_dir.join("linked.json")).unwrap(), b"existing-credential");
|
||||
fs::remove_dir_all(root).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn macos_relative_oauth_dir_preserves_populated_or_custom_paths() {
|
||||
let root = agent_test_home("macos-relative-oauth-dir-custom");
|
||||
|
||||
Reference in new issue
Block a user