mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Allow NFS database migration with shared file permissions (#57226)
## Why NFS database migration rejected files with group or other access. Allow accessible databases to migrate while making the effects on file access explicit. ## What changed - Preserve file mode bits during migration while requiring owner read and write permissions on replacement files. - Before CLI consent, disclose that replacements belong to the current user and do not copy the original owner, group, or extended ACLs. Require explicit confirmation for files with permissions other than `0600`. - Warn at Linux TUI startup when NFS databases need migration and permission confirmation. Inspect journal mode without accessing WAL shared memory or releasing another connection's locks. - Allow five seconds to acquire the exclusive database lock during migration. ## Testing Add CLI coverage for consent, access disclosures, and migration of `0644` files. Extend state tests for permission preservation, rejection of missing owner access, and interrupted migration recovery; snapshot the startup warning. GitOrigin-RevId: 2d9c3663172c3e631a787dc1dc76db95c39e6aa1
This commit is contained in:
committed by
copyberry
parent
480ab2d006
commit
ca8b7a0737
@@ -42,10 +42,6 @@ pub(crate) async fn run(
|
||||
eprintln!(
|
||||
"Stop all Codex instances on every host that uses these databases.\nKeep them stopped until the migration completes successfully."
|
||||
);
|
||||
ensure!(
|
||||
crate::confirm("Continue? [y/N]:")?,
|
||||
"Migration cancelled. No databases were changed."
|
||||
);
|
||||
let config = ConfigBuilder::default()
|
||||
.cli_overrides(
|
||||
config_overrides
|
||||
@@ -63,6 +59,31 @@ pub(crate) async fn run(
|
||||
println!("No databases found.");
|
||||
return Ok(());
|
||||
}
|
||||
let mut needs_permission_confirmation = false;
|
||||
for database in sqlite.runtime_db_paths() {
|
||||
if skip_logs && database.path == sqlite.logs_db_path() {
|
||||
continue;
|
||||
}
|
||||
needs_permission_confirmation |=
|
||||
NfsDbMigration::needs_permission_confirmation(&database.path)?;
|
||||
}
|
||||
if needs_permission_confirmation {
|
||||
eprintln!(
|
||||
"Some database files have permissions other than 0600.\nMigration preserves file mode bits, including group and other access."
|
||||
);
|
||||
}
|
||||
eprintln!(
|
||||
"Replacement files belong to you; the original owner, group, and extended ACLs are not copied.\nThis can remove another user's access even when file permissions are 0600.\nOriginal files remain in a private backup directory.\nYou need write access to the database directory and its migration lock."
|
||||
);
|
||||
let prompt = if needs_permission_confirmation {
|
||||
"Migrate databases, including files with permissions other than 0600? [y/N]:"
|
||||
} else {
|
||||
"Continue? [y/N]:"
|
||||
};
|
||||
ensure!(
|
||||
crate::confirm(prompt)?,
|
||||
"Migration cancelled. No databases were changed."
|
||||
);
|
||||
let mut migration = NfsDbMigration::lock(sqlite.home())?;
|
||||
for database in sqlite.runtime_db_paths() {
|
||||
if skip_logs && database.path == sqlite.logs_db_path() {
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
//! Exercise consent and file permissions through the migration CLI.
|
||||
|
||||
#![cfg(unix)]
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_state::SqliteConfig;
|
||||
use predicates::prelude::PredicateBooleanExt;
|
||||
use predicates::str::contains;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
#[tokio::test]
|
||||
async fn migration_requires_consent_and_preserves_shared_database_mode() -> Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
let sqlite = SqliteConfig::new_for_testing(home.path().to_path_buf().try_into()?);
|
||||
let path = sqlite.state_db_path();
|
||||
let pool = sqlite.open_read_write_pool(&path).await?;
|
||||
sqlx::raw_sql("CREATE TABLE entries (id INTEGER); INSERT INTO entries VALUES (42)")
|
||||
.execute(&pool)
|
||||
.await?;
|
||||
pool.close().await;
|
||||
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(/*mode*/ 0o600))?;
|
||||
let mut command = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?);
|
||||
let declined = command
|
||||
.env("CODEX_HOME", home.path())
|
||||
.env(codex_state::SQLITE_HOME_ENV, home.path())
|
||||
.arg("migrate-nfs-db")
|
||||
.write_stdin("n\n")
|
||||
.assert()
|
||||
.failure()
|
||||
.stderr(contains("permissions other than 0600").not())
|
||||
.stderr(contains("Continue? [y/N]"));
|
||||
let stderr = String::from_utf8_lossy(&declined.get_output().stderr);
|
||||
let disclosure = stderr
|
||||
.find("the original owner, group, and extended ACLs are not copied")
|
||||
.unwrap();
|
||||
let consent = stderr.find("Continue? [y/N]").unwrap();
|
||||
assert!(
|
||||
disclosure < consent,
|
||||
"ACL loss must be disclosed before consent, including for 0600 files"
|
||||
);
|
||||
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(/*mode*/ 0o644))?;
|
||||
let before = std::fs::read(&path)?;
|
||||
|
||||
let mut command = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?);
|
||||
command
|
||||
.env("CODEX_HOME", home.path())
|
||||
.env(codex_state::SQLITE_HOME_ENV, home.path())
|
||||
.arg("migrate-nfs-db")
|
||||
.write_stdin("n\n")
|
||||
.assert()
|
||||
.failure()
|
||||
.stderr(contains("permissions other than 0600"))
|
||||
.stderr(contains("No databases were changed"));
|
||||
assert_eq!(std::fs::read(&path)?, before);
|
||||
|
||||
let mut command = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?);
|
||||
command
|
||||
.env("CODEX_HOME", home.path())
|
||||
.env(codex_state::SQLITE_HOME_ENV, home.path())
|
||||
.arg("migrate-nfs-db")
|
||||
.write_stdin("y\n")
|
||||
.assert()
|
||||
.success()
|
||||
.stdout(contains("Migration completed successfully"));
|
||||
let pool = sqlite
|
||||
.open_read_only_pool(&path, /*busy_timeout*/ None)
|
||||
.await?;
|
||||
let mode: String = sqlx::query_scalar("PRAGMA journal_mode")
|
||||
.fetch_one(&pool)
|
||||
.await?;
|
||||
let rows: Vec<i64> = sqlx::query_scalar("SELECT id FROM entries")
|
||||
.fetch_all(&pool)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
(
|
||||
mode,
|
||||
rows,
|
||||
std::fs::metadata(&path)?.permissions().mode() & 0o777
|
||||
),
|
||||
("delete".to_string(), vec![42], 0o644)
|
||||
);
|
||||
pool.close().await;
|
||||
Ok(())
|
||||
}
|
||||
Reference in New Issue
Block a user