diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c796b5a98f..ae8e0bc20e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). tags. These keywords may be useful in non-colocated Git repositories where local and exported `@git` tags can point to different revisions. +* Per-repo configurations are now signed, preventing you from using + configurations created by another user. This means you must now use + `jj config edit` to edit in-repo configs instead of manually specifying the + path. + ### Fixed bugs * `jj metaedit --author-timestamp` twice with the same value no longer diff --git a/Cargo.lock b/Cargo.lock index 87c04a751a7..6e2a6c4a9a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -198,6 +198,12 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "base64ct" +version = "1.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55248b47b0caf0546f7988906588779981c43bb1bc9d0c44087278f80cdb44ba" + [[package]] name = "beef" version = "0.5.2" @@ -255,7 +261,7 @@ version = "0.10.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "46502ad458c9a52b69d4d4d32775c788b7a1b85e8bc9d482d92250fc0e3f8efe" dependencies = [ - "digest", + "digest 0.10.7", ] [[package]] @@ -267,6 +273,15 @@ dependencies = [ "generic-array", ] +[[package]] +name = "block-buffer" +version = "0.11.0-rc.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9ef36a6fcdb072aa548f3da057640ec10859eb4e91ddf526ee648d50c76a949" +dependencies = [ + "hybrid-array", +] + [[package]] name = "borrow-or-share" version = "0.2.2" @@ -524,6 +539,12 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "const-oid" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0dabb6555f92fb9ee4140454eb5dcd14c7960e1225c6d1a6cc361f032947713e" + [[package]] name = "core-foundation-sys" version = "0.8.7" @@ -659,6 +680,15 @@ dependencies = [ "typenum", ] +[[package]] +name = "crypto-common" +version = "0.2.0-rc.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a8235645834fbc6832939736ce2f2d08192652269e11010a6240f61b908a1c6" +dependencies = [ + "hybrid-array", +] + [[package]] name = "csscolorparser" version = "0.6.2" @@ -669,6 +699,33 @@ dependencies = [ "phf", ] +[[package]] +name = "curve25519-dalek" +version = "5.0.0-pre.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f9200d1d13637f15a6acb71e758f64624048d85b31a5fdbfd8eca1e2687d0b7" +dependencies = [ + "cfg-if", + "cpufeatures", + "curve25519-dalek-derive", + "digest 0.11.0-rc.3", + "fiat-crypto", + "rustc_version", + "subtle", + "zeroize", +] + +[[package]] +name = "curve25519-dalek-derive" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f46882e17999c6cc590af592290432be3bce0428cb0d5f8b6715e4dc7b383eb3" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.106", +] + [[package]] name = "darling" version = "0.20.11" @@ -736,6 +793,16 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5729f5117e208430e437df2f4843f5e5952997175992d1414f94c57d61e270b4" +[[package]] +name = "der" +version = "0.8.0-rc.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9d8dd2f26c86b27a2a8ea2767ec7f9df7a89516e4794e54ac01ee618dda3aa4" +dependencies = [ + "const-oid", + "zeroize", +] + [[package]] name = "diff" version = "0.1.13" @@ -754,11 +821,21 @@ version = "0.10.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ - "block-buffer", - "crypto-common", + "block-buffer 0.10.4", + "crypto-common 0.1.6", "subtle", ] +[[package]] +name = "digest" +version = "0.11.0-rc.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dac89f8a64533a9b0eaa73a68e424db0fb1fd6271c74cc0125336a05f090568d" +dependencies = [ + "block-buffer 0.11.0-rc.5", + "crypto-common 0.2.0-rc.4", +] + [[package]] name = "dirs" version = "6.0.0" @@ -803,6 +880,32 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" +[[package]] +name = "ed25519" +version = "3.0.0-rc.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ef49c0b20c0ad088893ad2a790a29c06a012b3f05bcfc66661fd22a94b32129" +dependencies = [ + "pkcs8", + "signature", +] + +[[package]] +name = "ed25519-dalek" +version = "3.0.0-pre.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ad207ed88a133091f83224265eac21109930db09bedcad05d5252f2af2de20a1" +dependencies = [ + "curve25519-dalek", + "ed25519", + "rand_core 0.9.3", + "serde", + "sha2 0.11.0-rc.2", + "signature", + "subtle", + "zeroize", +] + [[package]] name = "either" version = "1.15.0" @@ -946,6 +1049,12 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "fiat-crypto" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "64cd1e32ddd350061ae6edb1b082d7c54915b5c672c389143b9a63403a109f24" + [[package]] name = "filedescriptor" version = "0.8.3" @@ -2051,6 +2160,15 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "hybrid-array" +version = "0.4.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f471e0a81b2f90ffc0cb2f951ae04da57de8baa46fa99112b062a5173a5088d0" +dependencies = [ + "typenum", +] + [[package]] name = "iana-time-zone" version = "0.1.63" @@ -2425,6 +2543,7 @@ dependencies = [ "pollster", "proptest", "proptest-state-machine", + "prost", "rayon", "regex", "rpassword", @@ -2462,9 +2581,11 @@ dependencies = [ "chrono", "clru", "criterion", - "digest", + "digest 0.10.7", "dunce", + "ed25519-dalek", "either", + "etcetera", "futures 0.3.31", "gix", "globset", @@ -2482,6 +2603,7 @@ dependencies = [ "once_cell", "pest", "pest_derive", + "pkcs8", "pollster", "pretty_assertions", "proptest", @@ -3066,7 +3188,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72f27a2cfee9f9039c4d86faa5af122a0ac3851441a34865b8a043b46be0065a" dependencies = [ "pest", - "sha2", + "sha2 0.10.9", ] [[package]] @@ -3143,6 +3265,16 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" +[[package]] +name = "pkcs8" +version = "0.11.0-rc.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93eac55f10aceed84769df670ea4a32d2ffad7399400d41ee1c13b1cd8e1b478" +dependencies = [ + "der", + "spki", +] + [[package]] name = "plotters" version = "0.3.7" @@ -3823,7 +3955,7 @@ checksum = "e3bf829a2d51ab4a5ddf1352d8470c140cadc8301b2ae1789db023f01cedd6ba" dependencies = [ "cfg-if", "cpufeatures", - "digest", + "digest 0.10.7", ] [[package]] @@ -3832,7 +3964,7 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "89f599ac0c323ebb1c6082821a54962b839832b03984598375bff3975b804423" dependencies = [ - "digest", + "digest 0.10.7", "sha1", ] @@ -3844,7 +3976,18 @@ checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" dependencies = [ "cfg-if", "cpufeatures", - "digest", + "digest 0.10.7", +] + +[[package]] +name = "sha2" +version = "0.11.0-rc.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d1e3878ab0f98e35b2df35fe53201d088299b41a6bb63e3e34dada2ac4abd924" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest 0.11.0-rc.3", ] [[package]] @@ -3898,6 +4041,12 @@ dependencies = [ "libc", ] +[[package]] +name = "signature" +version = "3.0.0-rc.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc280a6ff65c79fbd6622f64d7127f32b85563bca8c53cd2e9141d6744a9056d" + [[package]] name = "similar" version = "2.7.0" @@ -3941,6 +4090,16 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "spki" +version = "0.8.0-rc.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8baeff88f34ed0691978ec34440140e1572b68c7dd4a495fd14a3dc1944daa80" +dependencies = [ + "base64ct", + "der", +] + [[package]] name = "stable_deref_trait" version = "1.2.0" @@ -4095,7 +4254,7 @@ dependencies = [ "pest", "pest_derive", "phf", - "sha2", + "sha2 0.10.9", "signal-hook", "siphasher", "terminfo", @@ -4792,7 +4951,7 @@ checksum = "692daff6d93d94e29e4114544ef6d5c942a7ed998b37abdc19b17136ea428eb7" dependencies = [ "getrandom 0.3.3", "mac_address", - "sha2", + "sha2 0.10.9", "thiserror 1.0.69", "uuid", ] @@ -5280,6 +5439,12 @@ dependencies = [ "synstructure", ] +[[package]] +name = "zeroize" +version = "1.8.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b97154e67e32c85465826e8bcc1c59429aaaf107c1e4a9e53c8d8ccd5eff88d0" + [[package]] name = "zerotrie" version = "0.2.2" diff --git a/Cargo.toml b/Cargo.toml index 9ab5472f6f5..71ab0c18ea2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ crossterm = { version = "0.28", default-features = false, features = ["windows"] datatest-stable = "0.3.3" digest = "0.10.7" dunce = "1.0.5" +ed25519-dalek = { version = "3.0.0-pre.1", features = ["alloc", "pkcs8", "rand_core"] } erased-serde = "0.4.8" etcetera = "0.10.0" either = "1.15.0" @@ -72,6 +73,7 @@ num_cpus = "1.17.0" once_cell = "1.21.3" pest = "2.8.3" pest_derive = "2.8.3" +pkcs8 = { version = "0.11.0-rc.7", features = ["std"] } pollster = "0.4.0" pretty_assertions = "1.4.1" proc-macro2 = "1.0.96" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index d59ee3109e4..8f203753198 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -82,6 +82,7 @@ once_cell = { workspace = true } pest = { workspace = true } pest_derive = { workspace = true } pollster = { workspace = true } +prost = { workspace = true } rayon = { workspace = true } regex = { workspace = true } rpassword = { workspace = true } diff --git a/cli/examples/custom-backend/main.rs b/cli/examples/custom-backend/main.rs index a8ec04e7cbc..3f0f76f7a41 100644 --- a/cli/examples/custom-backend/main.rs +++ b/cli/examples/custom-backend/main.rs @@ -66,14 +66,14 @@ fn create_store_factories() -> StoreFactories { } fn run_custom_command( - _ui: &mut Ui, + ui: &mut Ui, command_helper: &CommandHelper, command: CustomCommand, ) -> Result<(), CommandError> { match command { CustomCommand::InitJit => { let wc_path = command_helper.cwd(); - let settings = command_helper.settings_for_new_workspace(wc_path)?; + let settings = command_helper.settings_for_new_workspace(ui, wc_path)?; // Initialize a workspace with the custom backend Workspace::init_with_backend( &settings, diff --git a/cli/examples/custom-working-copy/main.rs b/cli/examples/custom-working-copy/main.rs index 9d12d490070..af9a80bcc07 100644 --- a/cli/examples/custom-working-copy/main.rs +++ b/cli/examples/custom-working-copy/main.rs @@ -56,14 +56,14 @@ enum CustomCommand { } fn run_custom_command( - _ui: &mut Ui, + ui: &mut Ui, command_helper: &CommandHelper, command: CustomCommand, ) -> Result<(), CommandError> { match command { CustomCommand::InitConflicts => { let wc_path = command_helper.cwd(); - let settings = command_helper.settings_for_new_workspace(wc_path)?; + let settings = command_helper.settings_for_new_workspace(ui, wc_path)?; let backend_initializer = |settings: &UserSettings, store_path: &Path| { let backend: Box = Box::new(GitBackend::init_internal(settings, store_path)?); diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 9d56a38ccf2..6770a0f358e 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -346,7 +346,7 @@ impl CommandHelper { /// /// This may be different from the settings for new workspace created by /// e.g. `jj git init`. There may be conditional variables and repo config - /// `.jj/repo/config.toml` loaded for the cwd workspace. + /// loaded for the cwd workspace. pub fn settings(&self) -> &UserSettings { &self.data.settings } @@ -354,14 +354,15 @@ impl CommandHelper { /// Resolves configuration for new workspace located at the specified path. pub fn settings_for_new_workspace( &self, + ui: &Ui, workspace_root: &Path, ) -> Result { let mut config_env = self.data.config_env.clone(); let mut raw_config = self.data.raw_config.clone(); let repo_path = workspace_root.join(".jj").join("repo"); - config_env.reset_repo_path(&repo_path); + config_env.reset_repo_path(ui, &repo_path)?; config_env.reload_repo_config(&mut raw_config)?; - config_env.reset_workspace_path(workspace_root); + config_env.reset_workspace_path(ui, workspace_root)?; config_env.reload_workspace_config(&mut raw_config)?; let mut config = config_env.resolve_config(&raw_config)?; // No migration messages here, which would usually be emitted before. @@ -3847,9 +3848,9 @@ impl<'a> CliRunner<'a> { .map_err(|err| map_workspace_load_error(err, Some("."))); config_env.reload_user_config(&mut raw_config)?; if let Ok(loader) = &maybe_cwd_workspace_loader { - config_env.reset_repo_path(loader.repo_path()); + config_env.reset_repo_path(ui, loader.repo_path())?; config_env.reload_repo_config(&mut raw_config)?; - config_env.reset_workspace_path(loader.workspace_root()); + config_env.reset_workspace_path(ui, loader.workspace_root())?; config_env.reload_workspace_config(&mut raw_config)?; } let mut config = config_env.resolve_config(&raw_config)?; @@ -3893,9 +3894,9 @@ impl<'a> CliRunner<'a> { .workspace_loader_factory .create(&abs_path) .map_err(|err| map_workspace_load_error(err, Some(path)))?; - config_env.reset_repo_path(loader.repo_path()); + config_env.reset_repo_path(ui, loader.repo_path())?; config_env.reload_repo_config(&mut raw_config)?; - config_env.reset_workspace_path(loader.workspace_root()); + config_env.reset_workspace_path(ui, loader.workspace_root())?; config_env.reload_workspace_config(&mut raw_config)?; Ok(loader) } else { diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 173e14ab1de..211bd506eb5 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -358,6 +358,10 @@ impl From for CommandError { } WorkspaceInitError::SignInit(err) => user_error(err), WorkspaceInitError::TransactionCommit(err) => err.into(), + WorkspaceInitError::UserConfigError(err) => internal_error_with_message( + "Failed to initialize user configuration for the repo", + err, + ), } } } diff --git a/cli/src/commands/config/edit.rs b/cli/src/commands/config/edit.rs index 3ca0ba9089a..81ee98dd74e 100644 --- a/cli/src/commands/config/edit.rs +++ b/cli/src/commands/config/edit.rs @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::sync::Arc; + +use jj_lib::config::ConfigFile; use jj_lib::config::ConfigLayer; use tracing::instrument; @@ -19,6 +22,7 @@ use super::ConfigLevelArgs; use crate::cli_util::CommandHelper; use crate::command_error::CommandError; use crate::command_error::print_error_sources; +use crate::commands::config::ConfigFileOrString; use crate::ui::Ui; /// Start an editor on a jj config file. @@ -40,36 +44,50 @@ pub fn cmd_config_edit( let editor = command.text_editor()?; let file = args.level.edit_config_file(ui, command)?; if !file.path().exists() { - file.save()?; + file.save(false)?; } // Editing again and again until either of these conditions is met // 1. The config is OK // 2. The user restores previous one - writeln!(ui.status(), "Editing file: {}", file.path().display())?; + if matches!(file, ConfigFileOrString::ConfigFile(_)) { + writeln!(ui.status(), "Editing file: {}", file.path().display())?; + } loop { editor.edit_file(file.path())?; // Trying to load back config. If error, prompt to continue editing - if let Err(e) = ConfigLayer::load_from_file(file.layer().source, file.path().to_path_buf()) - { - writeln!( - ui.warning_default(), - "An error has been found inside the config:" - )?; - print_error_sources(ui, Some(&e))?; - let continue_editing = ui.prompt_yes_no( - "Do you want to keep editing the file? If not, previous config will be restored.", - Some(true), - )?; - if !continue_editing { - // Saving back previous config - file.save()?; + match ConfigLayer::load_from_file(file.file().layer().source, file.path().to_path_buf()) { + Err(e) => { + writeln!( + ui.warning_default(), + "An error has been found inside the config:" + )?; + print_error_sources(ui, Some(&e))?; + let continue_editing = ui.prompt_yes_no( + "Do you want to keep editing the file? If not, previous config will be \ + restored.", + Some(true), + )?; + if !continue_editing { + // Saving back previous config + file.save(false)?; + break; + } + } + Ok(f) => { + // If it's a config string, it's backed by a temporary file. + // So we need to save it to ensure we don't lose it. + if let ConfigFileOrString::ConfigString(_, env, td) = file { + ConfigFileOrString::ConfigString( + ConfigFile::from_layer(Arc::new(f)).unwrap(), + env, + td, + ) + .save(true)?; + } break; } - } else { - // config is OK - break; } } Ok(()) diff --git a/cli/src/commands/config/mod.rs b/cli/src/commands/config/mod.rs index c0c741a713b..74ab74949b9 100644 --- a/cli/src/commands/config/mod.rs +++ b/cli/src/commands/config/mod.rs @@ -24,6 +24,7 @@ use std::path::Path; use itertools::Itertools as _; use jj_lib::config::ConfigFile; use jj_lib::config::ConfigSource; +use jj_lib::file_util::IoResultExt as _; use tracing::instrument; use self::edit::ConfigEditArgs; @@ -60,6 +61,40 @@ pub(crate) struct ConfigLevelArgs { workspace: bool, } +enum ConfigFileOrString<'a> { + ConfigFile(ConfigFile), + ConfigString(ConfigFile, &'a ConfigEnv, tempfile::TempDir), +} + +impl ConfigFileOrString<'_> { + fn file(&self) -> &ConfigFile { + match self { + Self::ConfigFile(f) | Self::ConfigString(f, ..) => f, + } + } + + fn file_mut(&mut self) -> &mut ConfigFile { + match self { + Self::ConfigFile(f) | Self::ConfigString(f, ..) => f, + } + } + + fn path(&self) -> &Path { + self.file().path() + } + + fn save(&self, trust: bool) -> Result<(), CommandError> { + match self { + Self::ConfigFile(f) => Ok(f.save()?), + Self::ConfigString(f, env, ..) => match f.layer().source { + ConfigSource::Repo => env.set_repo_config(&f.text(), trust), + ConfigSource::Workspace => env.set_workspace_config(&f.text(), trust), + _ => panic!("Bad config source"), + }, + } + } +} + impl ConfigLevelArgs { fn get_source_kind(&self) -> Option { if self.user { @@ -81,25 +116,23 @@ impl ConfigLevelArgs { } Ok(paths) } else if self.repo { - config_env - .repo_config_path() - .map(|p| vec![p]) - .ok_or_else(|| user_error("No repo config path found")) + Err(user_error( + "Repo configs are not present on disk. Use `jj config edit --repo", + )) } else if self.workspace { - config_env - .workspace_config_path() - .map(|p| vec![p]) - .ok_or_else(|| user_error("No workspace config path found")) + Err(user_error( + "Workspace configs are not present on disk. Use `jj config edit --workspace", + )) } else { panic!("No config_level provided") } } - fn edit_config_file( + fn edit_config_file<'a>( &self, ui: &Ui, - command: &CommandHelper, - ) -> Result { + command: &'a CommandHelper, + ) -> Result, CommandError> { let config_env = command.config_env(); let config = command.raw_config(); let pick_one = |mut files: Vec, not_found_error: &str| { @@ -118,20 +151,29 @@ impl ConfigLevelArgs { files.pop().ok_or_else(|| user_error(not_found_error)) }; if self.user { - pick_one( + Ok(ConfigFileOrString::ConfigFile(pick_one( config_env.user_config_files(config)?, "No user config path found to edit", - ) - } else if self.repo { - pick_one( - config_env.repo_config_files(config)?, - "No repo config path found to edit", - ) - } else if self.workspace { - pick_one( - config_env.workspace_config_files(config)?, - "No workspace config path found to edit", - ) + )?)) + } else if self.repo || self.workspace { + // Throw an error if we're not currently in a workspace. + command.workspace_loader()?; + let td = tempfile::TempDir::new()?; + let path = td.path().join("config.toml"); + let (content, source) = if self.repo { + (&config_env.repo_config().get().config, ConfigSource::Repo) + } else { + ( + &config_env.workspace_config().get().config, + ConfigSource::Workspace, + ) + }; + std::fs::write(&path, content).context(&path)?; + Ok(ConfigFileOrString::ConfigString( + ConfigFile::load_or_empty(source, &path)?, + config_env, + td, + )) } else { panic!("No config_level provided") } diff --git a/cli/src/commands/config/set.rs b/cli/src/commands/config/set.rs index b67355bb68b..547863669d3 100644 --- a/cli/src/commands/config/set.rs +++ b/cli/src/commands/config/set.rs @@ -75,9 +75,10 @@ pub fn cmd_config_set( check_wc_author(ui, command, &args.value, AuthorChange::Email)?; }; - file.set_value(&args.name, &args.value) + file.file_mut() + .set_value(&args.name, &args.value) .map_err(|err| user_error_with_message(format!("Failed to set {}", args.name), err))?; - file.save()?; + file.save(false)?; Ok(()) } diff --git a/cli/src/commands/config/unset.rs b/cli/src/commands/config/unset.rs index 0edbcc235b3..88dbc834446 100644 --- a/cli/src/commands/config/unset.rs +++ b/cli/src/commands/config/unset.rs @@ -41,11 +41,12 @@ pub fn cmd_config_unset( ) -> Result<(), CommandError> { let mut file = args.level.edit_config_file(ui, command)?; let old_value = file + .file_mut() .delete_value(&args.name) .map_err(|err| user_error_with_message(format!("Failed to unset {}", args.name), err))?; if old_value.is_none() { return Err(user_error(format!(r#""{}" doesn't exist"#, args.name))); } - file.save()?; + file.save(false)?; Ok(()) } diff --git a/cli/src/commands/debug/init_simple.rs b/cli/src/commands/debug/init_simple.rs index a2a8fe9671b..34287e3ea38 100644 --- a/cli/src/commands/debug/init_simple.rs +++ b/cli/src/commands/debug/init_simple.rs @@ -57,7 +57,7 @@ pub(crate) fn cmd_debug_init_simple( .and_then(|_| dunce::canonicalize(wc_path)) .map_err(|e| user_error_with_message("Failed to create workspace", e))?; - Workspace::init_simple(&command.settings_for_new_workspace(&wc_path)?, &wc_path)?; + Workspace::init_simple(&command.settings_for_new_workspace(ui, &wc_path)?, &wc_path)?; let relative_wc_path = file_util::relative_path(cwd, &wc_path); writeln!( diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs index 7383b8b801b..5c07a361ab6 100644 --- a/cli/src/commands/git/clone.rs +++ b/cli/src/commands/git/clone.rs @@ -211,7 +211,7 @@ fn init_workspace( wc_path: &Path, colocate: bool, ) -> Result { - let settings = command.settings_for_new_workspace(wc_path)?; + let settings = command.settings_for_new_workspace(ui, wc_path)?; let (workspace, repo) = if colocate { Workspace::init_colocated_git(&settings, wc_path)? } else { diff --git a/cli/src/commands/git/init.rs b/cli/src/commands/git/init.rs index 51f1fab50f5..82b59b4d230 100644 --- a/cli/src/commands/git/init.rs +++ b/cli/src/commands/git/init.rs @@ -168,7 +168,7 @@ fn do_init( GitInitMode::Internal }; - let settings = command.settings_for_new_workspace(workspace_root)?; + let settings = command.settings_for_new_workspace(ui, workspace_root)?; match &init_mode { GitInitMode::Colocate => { let (workspace, repo) = Workspace::init_colocated_git(&settings, workspace_root)?; diff --git a/cli/src/commands/git/mod.rs b/cli/src/commands/git/mod.rs index 274778b23e0..7b2d62e4527 100644 --- a/cli/src/commands/git/mod.rs +++ b/cli/src/commands/git/mod.rs @@ -21,7 +21,6 @@ mod push; mod remote; mod root; -use std::io::Write as _; use std::path::Path; use clap::Subcommand; @@ -30,9 +29,11 @@ use jj_lib::config::ConfigFile; use jj_lib::config::ConfigSource; use jj_lib::git; use jj_lib::git::UnexpectedGitBackendError; +use jj_lib::protos::user_config::RepoConfig; use jj_lib::ref_name::RemoteNameBuf; use jj_lib::ref_name::RemoteRefSymbol; use jj_lib::store::Store; +use jj_lib::user_config::write_user_config; use self::clone::GitCloneArgs; use self::clone::cmd_git_clone; @@ -53,6 +54,7 @@ use self::root::cmd_git_root; use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandHelper; use crate::command_error::CommandError; +use crate::command_error::internal_error; use crate::command_error::user_error_with_message; use crate::ui::Ui; @@ -127,7 +129,13 @@ fn write_repository_level_trunk_alias( let mut file = ConfigFile::load_or_empty(ConfigSource::Repo, repo_path.join("config.toml"))?; file.set_value(["revset-aliases", "trunk()"], symbol.to_string()) .expect("initial repo config shouldn't have invalid values"); - file.save()?; + write_user_config( + repo_path, + &RepoConfig { + config: file.text(), + }, + ) + .map_err(internal_error)?; writeln!( ui.status(), "Setting the revset alias `trunk()` to `{symbol}`", diff --git a/cli/src/complete.rs b/cli/src/complete.rs index 34162c79e58..8a3a646dab7 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -1010,9 +1010,9 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { let maybe_cwd_workspace_loader = DefaultWorkspaceLoaderFactory.create(find_workspace_dir(&cwd)); let _ = config_env.reload_user_config(&mut raw_config); if let Ok(loader) = &maybe_cwd_workspace_loader { - config_env.reset_repo_path(loader.repo_path()); + config_env.reset_repo_path(&ui, loader.repo_path())?; let _ = config_env.reload_repo_config(&mut raw_config); - config_env.reset_workspace_path(loader.workspace_root()); + config_env.reset_workspace_path(&ui, loader.workspace_root())?; let _ = config_env.reload_workspace_config(&mut raw_config); } let mut config = config_env.resolve_config(&raw_config)?; @@ -1030,9 +1030,9 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { if let Some(repository) = args.repository { // Try to update repo-specific config on a best-effort basis. if let Ok(loader) = DefaultWorkspaceLoaderFactory.create(&cwd.join(&repository)) { - config_env.reset_repo_path(loader.repo_path()); + config_env.reset_repo_path(&ui, loader.repo_path())?; let _ = config_env.reload_repo_config(&mut raw_config); - config_env.reset_workspace_path(loader.workspace_root()); + config_env.reset_workspace_path(&ui, loader.workspace_root())?; let _ = config_env.reload_workspace_config(&mut raw_config); if let Ok(new_config) = config_env.resolve_config(&raw_config) { config = new_config; diff --git a/cli/src/config.rs b/cli/src/config.rs index b6417d70b11..8fb556712ef 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -36,6 +36,14 @@ use jj_lib::config::ConfigSource; use jj_lib::config::ConfigValue; use jj_lib::config::StackedConfig; use jj_lib::dsl_util; +use jj_lib::file_util::IoResultExt as _; +use jj_lib::protos::user_config::RepoConfig; +use jj_lib::protos::user_config::WorkspaceConfig; +use jj_lib::user_config::ConfigType; +use jj_lib::user_config::UserConfig; +use jj_lib::user_config::UserConfigError; +use jj_lib::user_config::read_user_config; +use jj_lib::user_config::write_user_config; use regex::Captures; use regex::Regex; use serde::Serialize as _; @@ -44,6 +52,9 @@ use tracing::instrument; use crate::command_error::CommandError; use crate::command_error::config_error; use crate::command_error::config_error_with_message; +use crate::command_error::internal_error; +use crate::command_error::internal_error_with_message; +use crate::command_error::user_error_with_hint; use crate::text_util; use crate::ui::Ui; @@ -371,10 +382,10 @@ impl UnresolvedConfigEnv { pub struct ConfigEnv { home_dir: Option, repo_path: Option, + repo_config: UserConfig, workspace_path: Option, + workspace_config: UserConfig, user_config_paths: Vec, - repo_config_path: Option, - workspace_config_path: Option, command: Option, } @@ -418,10 +429,10 @@ impl ConfigEnv { Self { home_dir, repo_path: None, + repo_config: UserConfig::Trusted(Default::default()), workspace_path: None, + workspace_config: UserConfig::Trusted(Default::default()), user_config_paths: env.resolve(ui), - repo_config_path: None, - workspace_config_path: None, command: None, } } @@ -488,45 +499,112 @@ impl ConfigEnv { Ok(()) } + fn load_user_config( + &mut self, + ui: &Ui, + dir: &Path, + ) -> Result, CommandError> { + let user_config = match read_user_config::(dir) { + Err(UserConfigError::LegacyRepo) => { + write_user_config(dir, &T::default()).map_err(|e| { + internal_error_with_message("Failed to migrate from legacy repo", e) + })?; + return Ok(UserConfig::Trusted(Default::default())); + } + other => other, + } + .map_err(internal_error)?; + if let UserConfig::RepoMoved { from, to, .. } = &user_config { + let kind = T::kind(); + writeln!( + ui.warning_default(), + "The {kind} has moved from {from} to {to}. For security reasons, we have disabled \ + the {kind} config.", + )?; + writeln!( + ui.hint_default(), + "Run `jj config edit --{kind}` to review and re-enable" + )?; + } else if matches!(user_config, UserConfig::InvalidSignature(_)) { + let kind = T::kind(); + writeln!( + ui.warning_default(), + "The {kind} appears to have been created by someone else. For security reasons, \ + we have disabled the {kind} config.", + )?; + writeln!( + ui.hint_default(), + "Run `jj config edit --{kind}` to review and re-enable" + )?; + } + Ok(user_config) + } + /// Sets the directory where repo-specific config file is stored. The path /// is usually `.jj/repo`. - pub fn reset_repo_path(&mut self, path: &Path) { + pub fn reset_repo_path(&mut self, ui: &Ui, path: &Path) -> Result<(), CommandError> { self.repo_path = Some(path.to_owned()); - self.repo_config_path = Some(ConfigPath::new(path.join("config.toml"))); + // Provide a grace period for which repos without secure user config + // information will be automatically migrated to add it. + // If we did not provide this grace period, repo configs would be + // disabled for every repo created by an older version of jj. + // TODO: After the grace period is over (~jj 0.47), make this act the + // same as InvalidSignature or RepoMovedError . + let legacy_config = path.join("config.toml"); + match std::fs::read_to_string(&legacy_config).context(&legacy_config) { + Ok(content) => match read_user_config::(path) { + Ok(_) => { + return Err(user_error_with_hint( + "Both new and legacy repo config were found. You need to delete .jj/repo/config.toml.", + "You probably want to copy the contents of .jj/repo/config.toml and run `jj config edit --repo` to edit your repo config instead", + )); + } + Err(UserConfigError::LegacyRepo) => { + writeln!( + ui.warning_default(), + "Migrating from legacy repo config file (`$REPO_DIR/config.toml`) to \ + secure configuration", + )?; + writeln!( + ui.hint_default(), + "In the future, use `jj config edit --repo` to edit your config files \ + instead" + )?; + self.set_repo_config(&content, true)?; + std::fs::remove_file(&legacy_config).context(&legacy_config)?; + } + Err(e) => return Err(internal_error(e)), + }, + Err(e) if e.source.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + + self.repo_config = self.load_user_config::(ui, path)?; + Ok(()) } /// Returns a path to the repo-specific config file. - pub fn repo_config_path(&self) -> Option<&Path> { - self.repo_config_path.as_ref().map(|p| p.as_path()) + pub fn repo_config(&self) -> &UserConfig { + &self.repo_config } - /// Returns a path to the existing repo-specific config file. - fn existing_repo_config_path(&self) -> Option<&Path> { - match self.repo_config_path { - Some(ref path) if path.exists() => Some(path.as_path()), - _ => None, + pub fn set_repo_config(&self, config: &str, trusted: bool) -> Result<(), CommandError> { + if !trusted && !matches!(self.repo_config, UserConfig::Trusted(_)) { + return Err(user_error_with_hint( + "Attempting to update a non-trusted repo configuration", + "Run `jj config edit --repo` first", + )); } - } - - /// Returns repo configuration files for modification. Instantiates one if - /// `config` has no repo configuration layers. - /// - /// If the repo path is unknown, this function returns an empty `Vec`. Since - /// the repo config path cannot be a directory, the returned `Vec` should - /// have at most one config file. - pub fn repo_config_files( - &self, - config: &RawConfig, - ) -> Result, ConfigLoadError> { - config_files_for(config, ConfigSource::Repo, || self.new_repo_config_file()) - } - - fn new_repo_config_file(&self) -> Result, ConfigLoadError> { - self.repo_config_path() - // The path doesn't usually exist, but we shouldn't overwrite it - // with an empty config if it did exist. - .map(|path| ConfigFile::load_or_empty(ConfigSource::Repo, path)) - .transpose() + if let Some(repo_path) = self.repo_path.as_deref() { + write_user_config::( + repo_path, + &RepoConfig { + config: config.to_owned(), + }, + ) + .map_err(|e| internal_error_with_message("Failed to update repo config", e))?; + } + Ok(()) } /// Loads repo-specific config file into the given `config`. The old @@ -534,53 +612,43 @@ impl ConfigEnv { #[instrument] pub fn reload_repo_config(&self, config: &mut RawConfig) -> Result<(), ConfigLoadError> { config.as_mut().remove_layers(ConfigSource::Repo); - if let Some(path) = self.existing_repo_config_path() { - config.as_mut().load_file(ConfigSource::Repo, path)?; + if let UserConfig::Trusted(cfg) = &self.repo_config { + config + .as_mut() + .add_layer(ConfigLayer::parse(ConfigSource::Repo, &cfg.config)?); } Ok(()) } /// Sets the directory for the workspace and the workspace-specific config /// file. - pub fn reset_workspace_path(&mut self, path: &Path) { + pub fn reset_workspace_path(&mut self, ui: &Ui, path: &Path) -> Result<(), CommandError> { self.workspace_path = Some(path.to_owned()); - self.workspace_config_path = Some(ConfigPath::new( - path.join(".jj").join("workspace-config.toml"), - )); + self.workspace_config = self.load_user_config::(ui, &path.join(".jj"))?; + Ok(()) } - /// Returns a path to the workspace-specific config file. - pub fn workspace_config_path(&self) -> Option<&Path> { - self.workspace_config_path.as_ref().map(|p| p.as_path()) + pub fn workspace_config(&self) -> &UserConfig { + &self.workspace_config } - /// Returns a path to the existing workspace-specific config file. - fn existing_workspace_config_path(&self) -> Option<&Path> { - match self.workspace_config_path { - Some(ref path) if path.exists() => Some(path.as_path()), - _ => None, + pub fn set_workspace_config(&self, config: &str, trusted: bool) -> Result<(), CommandError> { + if !trusted && !matches!(self.workspace_config, UserConfig::Trusted(_)) { + return Err(user_error_with_hint( + "Attempting to update a non-trusted repo configuration", + "Run `jj config edit --repo` first", + )); } - } - - /// Returns workspace configuration files for modification. Instantiates one - /// if `config` has no workspace configuration layers. - /// - /// If the workspace path is unknown, this function returns an empty `Vec`. - /// Since the workspace config path cannot be a directory, the returned - /// `Vec` should have at most one config file. - pub fn workspace_config_files( - &self, - config: &RawConfig, - ) -> Result, ConfigLoadError> { - config_files_for(config, ConfigSource::Workspace, || { - self.new_workspace_config_file() - }) - } - - fn new_workspace_config_file(&self) -> Result, ConfigLoadError> { - self.workspace_config_path() - .map(|path| ConfigFile::load_or_empty(ConfigSource::Workspace, path)) - .transpose() + if let Some(workspace_path) = self.workspace_path.as_deref() { + write_user_config::( + &workspace_path.join(".jj"), + &WorkspaceConfig { + config: config.to_owned(), + }, + ) + .map_err(|e| internal_error_with_message("Failed to update workspace config", e))?; + } + Ok(()) } /// Loads workspace-specific config file into the given `config`. The old @@ -588,8 +656,10 @@ impl ConfigEnv { #[instrument] pub fn reload_workspace_config(&self, config: &mut RawConfig) -> Result<(), ConfigLoadError> { config.as_mut().remove_layers(ConfigSource::Workspace); - if let Some(path) = self.existing_workspace_config_path() { - config.as_mut().load_file(ConfigSource::Workspace, path)?; + if let UserConfig::Trusted(cfg) = &self.workspace_config { + config + .as_mut() + .add_layer(ConfigLayer::parse(ConfigSource::Workspace, &cfg.config)?); } Ok(()) } @@ -631,8 +701,8 @@ fn config_files_for( /// 1. Default /// 2. Base environment variables /// 3. [User configs](https://jj-vcs.github.io/jj/latest/config/) -/// 4. Repo config `.jj/repo/config.toml` -/// 5. Workspace config `.jj/workspace-config.toml` +/// 4. Repo config +/// 5. Workspace config /// 6. Override environment variables /// 7. Command-line arguments `--config` and `--config-file` /// @@ -1847,10 +1917,10 @@ mod tests { ConfigEnv { home_dir, repo_path: None, + repo_config: UserConfig::Trusted(Default::default()), workspace_path: None, + workspace_config: UserConfig::Trusted(Default::default()), user_config_paths: env.resolve(&Ui::null()), - repo_config_path: None, - workspace_config_path: None, command: None, } } diff --git a/cli/tests/common/test_environment.rs b/cli/tests/common/test_environment.rs index 7eb2aca6000..16d29a81b30 100644 --- a/cli/tests/common/test_environment.rs +++ b/cli/tests/common/test_environment.rs @@ -143,6 +143,16 @@ impl TestEnvironment { cmd.env("JJ_OP_TIMESTAMP", timestamp.to_rfc3339()); if cfg!(windows) { + // On windows, etcetera's data_dir function attempts to use APPDATA. + // This prevents it from attempting to use the real APPDATA. + cmd.env( + "APPDATA", + self.home_dir + .join("Appdata") + .join("Roaming") + .to_str() + .unwrap(), + ); // Windows uses `TEMP` to create temporary directories, which we need for some // tests. if let Ok(tmp_var) = std::env::var("TEMP") { diff --git a/cli/tests/runner.rs b/cli/tests/runner.rs index 22cac99be2b..551f9faa789 100644 --- a/cli/tests/runner.rs +++ b/cli/tests/runner.rs @@ -22,6 +22,7 @@ mod test_completion; mod test_concurrent_operations; mod test_config_command; mod test_config_schema; +mod test_config_security; mod test_copy_detection; mod test_debug_command; mod test_debug_init_simple_command; diff --git a/cli/tests/test_alias.rs b/cli/tests/test_alias.rs index b51b84773ff..83ca7c150a4 100644 --- a/cli/tests/test_alias.rs +++ b/cli/tests/test_alias.rs @@ -308,10 +308,15 @@ fn test_alias_in_repo_config() { work_dir2.create_dir("sub"); test_env.add_config(r#"aliases.l = ['log', '-r@', '--no-graph', '-T"user alias\n"']"#); - work_dir1.write_file( - ".jj/repo/config.toml", - r#"aliases.l = ['log', '-r@', '--no-graph', '-T"repo1 alias\n"']"#, - ); + work_dir1 + .run_jj([ + "config", + "set", + "--repo", + "aliases.l", + r#"['log', '-r@', '--no-graph', '-T"repo1 alias\n"']"#, + ]) + .success(); // In repo1 sub directory, aliases can be loaded from the repo1 config. let output = test_env.run_jj_in(work_dir1.root().join("sub"), ["l"]); diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index 23c4746105d..411f6e2a43e 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -330,9 +330,9 @@ fn test_config_list_origin() { "--config", "test-cli-key=test-cli-val", ]); - insta::assert_snapshot!(output, @r#" + insta::assert_snapshot!(output, @r###" test-key = "test-val" # user $TEST_ENV/config/config.toml - test-layered-key = "test-layered-val" # repo $TEST_ENV/repo/.jj/repo/config.toml + test-layered-key = "test-layered-val" # repo user.name = "Test User" # env user.email = "test.user@example.com" # env debug.commit-timestamp = "2001-02-03T04:05:11+07:00" # env @@ -342,7 +342,7 @@ fn test_config_list_origin() { operation.username = "test-username" # env test-cli-key = "test-cli-val" # cli [EOF] - "#); + "###); let output = work_dir.run_jj([ "config", @@ -408,10 +408,15 @@ fn test_config_layer_override_default() { "#); // Repo - work_dir.write_file( - ".jj/repo/config.toml", - format!("{config_key} = {value}\n", value = to_toml_value("repo")), - ); + work_dir + .run_jj([ + "config", + "set", + "--repo", + config_key, + &format!("{}", to_toml_value("repo")), + ]) + .success(); let output = work_dir.run_jj(["config", "list", config_key]); insta::assert_snapshot!(output, @r#" merge-tools.vimdiff.program = "repo" @@ -488,10 +493,15 @@ fn test_config_layer_override_env() { "#); // Repo - work_dir.write_file( - ".jj/repo/config.toml", - format!("{config_key} = {value}\n", value = to_toml_value("repo")), - ); + work_dir + .run_jj([ + "config", + "set", + "--repo", + config_key, + &format!("{}", to_toml_value("repo")), + ]) + .success(); let output = work_dir.run_jj(["config", "list", config_key]); insta::assert_snapshot!(output, @r#" ui.editor = "repo" @@ -554,13 +564,15 @@ fn test_config_layer_workspace() { .success(); // Repo - main_dir.write_file( - ".jj/repo/config.toml", - format!( - "{config_key} = {value}\n", - value = to_toml_value("main-repo") - ), - ); + main_dir + .run_jj([ + "config", + "set", + "--repo", + config_key, + &format!("{}", to_toml_value("main-repo")), + ]) + .success(); let output = main_dir.run_jj(["config", "list", config_key]); insta::assert_snapshot!(output, @r#" ui.editor = "main-repo" @@ -716,41 +728,49 @@ fn test_config_set_for_repo() { work_dir .run_jj(["config", "set", "--repo", "test-table.foo", "true"]) .success(); - // Ensure test-key successfully written to user config. - let repo_config_toml = work_dir.read_file(".jj/repo/config.toml"); - insta::assert_snapshot!(repo_config_toml, @r#" - #:schema https://jj-vcs.github.io/jj/latest/config-schema.json - - test-key = "test-val" - [test-table] - foo = true - "#); + let output = work_dir.run_jj(["config", "get", "test-key"]); + insta::assert_snapshot!(output, @r###" + test-val + [EOF] + "###); + let output = work_dir.run_jj(["config", "get", "test-table.foo"]); + insta::assert_snapshot!(output, @r###" + true + [EOF] + "###); } #[test] fn test_config_set_for_workspace() { let test_env = TestEnvironment::default(); test_env.run_jj_in(".", ["git", "init", "repo"]).success(); - let work_dir = test_env.work_dir("repo"); - work_dir.run_jj(["new"]).success(); - work_dir + let repo_dir = test_env.work_dir("repo"); + repo_dir.run_jj(["new"]).success(); + repo_dir .run_jj(["workspace", "add", "--name", "second", "../secondary"]) .success(); - let work_dir = test_env.work_dir("secondary"); + let ws_dir = test_env.work_dir("secondary"); // set in workspace - work_dir + ws_dir .run_jj(["config", "set", "--workspace", "test-key", "ws-val"]) .success(); - // Read workspace config - let workspace_config = work_dir.read_file(".jj/workspace-config.toml"); - insta::assert_snapshot!(workspace_config, @r#" - #:schema https://jj-vcs.github.io/jj/latest/config-schema.json + let output = ws_dir.run_jj(["config", "get", "test-key"]); + insta::assert_snapshot!(output, @r###" + ws-val + [EOF] + "###); - test-key = "ws-val" - "#); + let output = repo_dir.run_jj(["config", "get", "test-key"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Config error: Value not found for test-key + For help, see https://jj-vcs.github.io/jj/latest/config/ or use `jj help -k config`. + [EOF] + [exit status: 1] + "###); } #[test] @@ -957,35 +977,68 @@ fn test_config_unset_for_repo() { work_dir .run_jj(["config", "set", "--repo", "test-key", "test-val"]) .success(); + let output = work_dir.run_jj(["config", "get", "test-key"]); + insta::assert_snapshot!(output, @r###" + test-val + [EOF] + "###); + work_dir .run_jj(["config", "unset", "--repo", "test-key"]) .success(); - - let repo_config_toml = work_dir.read_file(".jj/repo/config.toml"); - insta::assert_snapshot!(repo_config_toml, @""); + let output = work_dir.run_jj(["config", "get", "test-key"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Config error: Value not found for test-key + For help, see https://jj-vcs.github.io/jj/latest/config/ or use `jj help -k config`. + [EOF] + [exit status: 1] + "###); } #[test] fn test_config_unset_for_workspace() { let test_env = TestEnvironment::default(); test_env.run_jj_in(".", ["git", "init", "repo"]).success(); - let work_dir = test_env.work_dir("repo"); - work_dir.run_jj(["new"]).success(); - work_dir + let repo_dir = test_env.work_dir("repo"); + repo_dir.run_jj(["new"]).success(); + repo_dir .run_jj(["workspace", "add", "--name", "second", "../secondary"]) .success(); - let work_dir = test_env.work_dir("secondary"); + let ws_dir = test_env.work_dir("secondary"); // set then unset - work_dir + ws_dir .run_jj(["config", "set", "--workspace", "foo", "bar"]) .success(); - work_dir + + let output = ws_dir.run_jj(["config", "get", "foo"]); + insta::assert_snapshot!(output, @r###" + bar + [EOF] + "###); + + let output = repo_dir.run_jj(["config", "get", "foo"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Config error: Value not found for foo + For help, see https://jj-vcs.github.io/jj/latest/config/ or use `jj help -k config`. + [EOF] + [exit status: 1] + "###); + + ws_dir .run_jj(["config", "unset", "--workspace", "foo"]) .success(); - let workspace_config = work_dir.read_file(".jj/workspace-config.toml"); - insta::assert_snapshot!(workspace_config, @""); + let output = ws_dir.run_jj(["config", "get", "foo"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Config error: Value not found for foo + For help, see https://jj-vcs.github.io/jj/latest/config/ or use `jj help -k config`. + [EOF] + [exit status: 1] + "###); } #[test] @@ -1120,18 +1173,15 @@ fn test_config_edit_repo() { let edit_script = test_env.set_up_fake_editor(); test_env.run_jj_in(".", ["git", "init", "repo"]).success(); let work_dir = test_env.work_dir("repo"); - let repo_config_path = work_dir - .root() - .join(PathBuf::from_iter([".jj", "repo", "config.toml"])); - assert!(!repo_config_path.exists()); - - std::fs::write(edit_script, "dump-path path").unwrap(); + std::fs::write(edit_script, "write\nfoo = \"bar\"").unwrap(); work_dir.run_jj(["config", "edit", "--repo"]).success(); - let edited_path = - PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap()); - assert_eq!(edited_path, dunce::simplified(&repo_config_path)); - assert!(repo_config_path.exists(), "new file should be created"); + let output = work_dir.run_jj(["config", "get", "foo"]); + insta::assert_snapshot!(output, @r###" + bar + [EOF] + "### + ); } #[test] @@ -1152,9 +1202,8 @@ fn test_config_edit_invalid_config() { .args(["config", "edit", "--repo"]) .write_stdin("Y\n") }); - insta::assert_snapshot!(output, @r" + insta::assert_snapshot!(output, @r###" ------- stderr ------- - Editing file: $TEST_ENV/repo/.jj/repo/config.toml Warning: An error has been found inside the config: Caused by: 1: Configuration cannot be parsed as TOML document @@ -1165,7 +1214,7 @@ fn test_config_edit_invalid_config() { key with no value, expected `=` Do you want to keep editing the file? If not, previous config will be restored. (Yn): [EOF] - "); + "###); let output = work_dir.run_jj(["config", "get", "test"]); insta::assert_snapshot!(output, @r" @@ -1181,9 +1230,8 @@ fn test_config_edit_invalid_config() { .args(["config", "edit", "--repo"]) .write_stdin("n\n") }); - insta::assert_snapshot!(output, @r" + insta::assert_snapshot!(output, @r###" ------- stderr ------- - Editing file: $TEST_ENV/repo/.jj/repo/config.toml Warning: An error has been found inside the config: Caused by: 1: Configuration cannot be parsed as TOML document @@ -1194,7 +1242,7 @@ fn test_config_edit_invalid_config() { key with no value, expected `=` Do you want to keep editing the file? If not, previous config will be restored. (Yn): [EOF] - "); + "###); let output = work_dir.run_jj(["config", "get", "test"]); insta::assert_snapshot!(output, @r" @@ -1225,21 +1273,23 @@ fn test_config_path() { "jj config path shouldn't create new file" ); - insta::assert_snapshot!(work_dir.run_jj(["config", "path", "--repo"]), @r" - $TEST_ENV/repo/.jj/repo/config.toml + insta::assert_snapshot!(work_dir.run_jj(["config", "path", "--repo"]), @r###" + ------- stderr ------- + Error: Repo configs are not present on disk. Use `jj config edit --repo [EOF] - "); + [exit status: 1] + "###); assert!( !repo_config_path.exists(), "jj config path shouldn't create new file" ); - insta::assert_snapshot!(test_env.run_jj_in(".", ["config", "path", "--repo"]), @r" + insta::assert_snapshot!(test_env.run_jj_in(".", ["config", "path", "--repo"]), @r###" ------- stderr ------- - Error: No repo config path found + Error: Repo configs are not present on disk. Use `jj config edit --repo [EOF] [exit status: 1] - "); + "###); } #[test] @@ -1277,12 +1327,12 @@ fn test_config_only_loads_toml_files() { fn test_config_edit_repo_outside_repo() { let test_env = TestEnvironment::default(); let output = test_env.run_jj_in(".", ["config", "edit", "--repo"]); - insta::assert_snapshot!(output, @r" + insta::assert_snapshot!(output, @r###" ------- stderr ------- - Error: No repo config path found to edit + Error: There is no jj repo in "." [EOF] [exit status: 1] - "); + "###); } #[test] diff --git a/cli/tests/test_config_security.rs b/cli/tests/test_config_security.rs new file mode 100644 index 00000000000..a38f160cc48 --- /dev/null +++ b/cli/tests/test_config_security.rs @@ -0,0 +1,218 @@ +// Copyright 2024 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::path::Path; + +use jj_lib::protos::user_config::RepoConfig; +use jj_lib::user_config::ConfigType as _; + +use crate::common::TestEnvironment; + +fn copy_dir_all(src: &Path, dst: &Path) -> std::io::Result<()> { + std::fs::create_dir_all(dst)?; + for entry in std::fs::read_dir(src)? { + let entry = entry?; + let ty = entry.file_type()?; + if ty.is_dir() { + copy_dir_all(&entry.path(), &dst.join(entry.file_name()))?; + } else { + std::fs::copy(entry.path(), dst.join(entry.file_name()))?; + } + } + Ok(()) +} + +// Here we simulate an attacker attempting to send you a zip file containing a +// malicious jj repo +#[test] +fn test_attacker_without_signature() { + // A test environment representing the user under attack. + let mut victim_env = TestEnvironment::default(); + // A test environment with a different home directory, representing a malicious + // actor. + let evil_env = TestEnvironment::default(); + let work_dir = evil_env.work_dir("").create_dir("repo"); + evil_env.run_jj_in(".", ["git", "init", "repo"]).success(); + + // We see that the config was created by someone else, but it was empty so + // it doesn't pose a security risk. + let d = work_dir.root().canonicalize().unwrap(); + let output = victim_env.run_jj_in(&d, ["status"]); + insta::assert_snapshot!(output, @r###" + The working copy has no changes. + Working copy (@) : qpvuntsm e8849ae1 (empty) (no description set) + Parent commit (@-): zzzzzzzz 00000000 (empty) (no description set) + [EOF] + "###); + + evil_env + .run_jj_in(&d, ["config", "set", "--repo", "repo-key", "repo-val"]) + .success(); + let output = victim_env.run_jj_in(&d, ["status"]); + insta::assert_snapshot!(output, @r###" + The working copy has no changes. + Working copy (@) : qpvuntsm e8849ae1 (empty) (no description set) + Parent commit (@-): zzzzzzzz 00000000 (empty) (no description set) + [EOF] + ------- stderr ------- + Warning: The repo appears to have been created by someone else. For security reasons, we have disabled the repo config. + Hint: Run `jj config edit --repo` to review and re-enable + [EOF] + "###); + + evil_env + .run_jj_in( + &d, + [ + "config", + "set", + "--workspace", + "workspace-key", + "workspace-val", + ], + ) + .success(); + + let output = victim_env.run_jj_in(&d, ["config", "get", "workspace-key"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Warning: The repo appears to have been created by someone else. For security reasons, we have disabled the repo config. + Hint: Run `jj config edit --repo` to review and re-enable + Warning: The workspace appears to have been created by someone else. For security reasons, we have disabled the workspace config. + Hint: Run `jj config edit --workspace` to review and re-enable + Config error: Value not found for workspace-key + For help, see https://jj-vcs.github.io/jj/latest/config/ or use `jj help -k config`. + [EOF] + [exit status: 1] + "###); + + // The victim now reviews the repo config and finds it to not be a security + // risk. + let edit_script = victim_env.set_up_fake_editor(); + victim_env + .run_jj_in(&d, ["config", "edit", "--repo"]) + .success(); + let output = victim_env.run_jj_in(&d, ["config", "get", "repo-key"]); + insta::assert_snapshot!(output, @r###" + repo-val + [EOF] + ------- stderr ------- + Warning: The workspace appears to have been created by someone else. For security reasons, we have disabled the workspace config. + Hint: Run `jj config edit --workspace` to review and re-enable + [EOF] + "###); + + // The victim now reviews the workspace config and finds it to be a security + // risk, so makes some changes. + std::fs::write(&edit_script, "write\nworkspace-key = \"new-workspace-val\"").unwrap(); + victim_env + .run_jj_in(&d, ["config", "edit", "--workspace"]) + .success(); + let output = victim_env.run_jj_in(&d, ["config", "get", "workspace-key"]); + insta::assert_snapshot!(output, @r###" + new-workspace-val + [EOF] + "###); +} + +// Here, we simulate you having sent the attacker a zip file. +// They then send you another repo with that signature and you download it to a +// different directory. +#[test] +fn test_attacker_has_signature() { + let test_env = TestEnvironment::default(); + let repo_dir = test_env.work_dir("").create_dir("repo"); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + + let evil_dir = test_env.work_dir("evil"); + copy_dir_all(repo_dir.root(), evil_dir.root()).unwrap(); + + // The file has moved but there's no config, so we consider it safe. + let output = evil_dir.run_jj(["status"]); + insta::assert_snapshot!(output, @r###" + The working copy has no changes. + Working copy (@) : qpvuntsm e8849ae1 (empty) (no description set) + Parent commit (@-): zzzzzzzz 00000000 (empty) (no description set) + [EOF] + "###); + + repo_dir + .run_jj(["config", "set", "--repo", "repo-key", "repo-val"]) + .success(); + repo_dir + .run_jj([ + "config", + "set", + "--workspace", + "workspace-key", + "workspace-val", + ]) + .success(); + + std::fs::remove_dir_all(evil_dir.root()).unwrap(); + copy_dir_all(repo_dir.root(), evil_dir.root()).unwrap(); + + // It's no longer safe because it now has config. + let output = test_env.run_jj_in(evil_dir.root(), ["config", "get", "repo-key"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Warning: The repo has moved from $TEST_ENV/repo/.jj/repo to $TEST_ENV/evil/.jj/repo. For security reasons, we have disabled the repo config. + Hint: Run `jj config edit --repo` to review and re-enable + Warning: The workspace has moved from $TEST_ENV/repo/.jj to $TEST_ENV/evil/.jj. For security reasons, we have disabled the workspace config. + Hint: Run `jj config edit --workspace` to review and re-enable + Config error: Value not found for repo-key + For help, see https://jj-vcs.github.io/jj/latest/config/ or use `jj help -k config`. + [EOF] + [exit status: 1] + "###); +} + +#[test] +fn test_legacy_config_migration() { + let test_env = TestEnvironment::default(); + let work_dir = test_env.work_dir("").create_dir("repo"); + work_dir.run_jj(["git", "init"]).success(); + + // Convert it to a legacy repo. + let legacy_config = work_dir.root().join(".jj/repo/config.toml"); + let new_config = work_dir + .root() + .join(".jj/repo") + .join(RepoConfig::filename()); + std::fs::write(&legacy_config, "foo = \"bar\"").unwrap(); + std::fs::remove_file(&new_config).unwrap(); + + let output = work_dir.run_jj(["config", "get", "foo"]); + insta::assert_snapshot!(output, @r###" + bar + [EOF] + ------- stderr ------- + Warning: Migrating from legacy repo config file (`$REPO_DIR/config.toml`) to secure configuration + Hint: In the future, use `jj config edit --repo` to edit your config files instead + [EOF] + "###); + + assert!(!legacy_config.exists()); + assert!(new_config.exists()); + + std::fs::write(&legacy_config, "foo = \"baz\"").unwrap(); + let output = work_dir.run_jj(["config", "get", "foo"]); + insta::assert_snapshot!(output, @r###" + ------- stderr ------- + Error: Both new and legacy repo config were found. You need to delete .jj/repo/config.toml. + Hint: You probably want to copy the contents of .jj/repo/config.toml and run `jj config edit --repo` to edit your repo config instead + [EOF] + [exit status: 1] + "###); +} diff --git a/cli/tests/test_git_clone.rs b/cli/tests/test_git_clone.rs index 27449ee422f..462440988f5 100644 --- a/cli/tests/test_git_clone.rs +++ b/cli/tests/test_git_clone.rs @@ -246,7 +246,7 @@ fn test_git_clone_colocate() { ); // ".jj" directory should be ignored at Git side. let git_statuses = git::status(&jj_git_repo); - insta::assert_debug_snapshot!(git_statuses, @r#" + insta::assert_debug_snapshot!(git_statuses, @r###" [ GitStatus { path: ".jj/.gitignore", @@ -266,8 +266,14 @@ fn test_git_clone_colocate() { Ignored, ), }, + GitStatus { + path: ".jj/workspace-config.binpb", + status: Worktree( + Ignored, + ), + }, ] - "#); + "###); // The old default bookmark "master" shouldn't exist. insta::assert_snapshot!(get_bookmark_output(&clone_dir), @r" diff --git a/cli/tests/test_git_init.rs b/cli/tests/test_git_init.rs index ca55e24f8db..20915ab97cd 100644 --- a/cli/tests/test_git_init.rs +++ b/cli/tests/test_git_init.rs @@ -739,7 +739,7 @@ fn test_git_init_colocated_dirty_working_copy() { // current implementation, the index is unchanged. Since jj created new // working copy commit, it's also okay to update the index reflecting the // working copy commit or the working copy parent. - insta::assert_debug_snapshot!(git::status(&git_repo), @r#" + insta::assert_debug_snapshot!(git::status(&git_repo), @r###" [ GitStatus { path: ".jj/.gitignore", @@ -759,6 +759,12 @@ fn test_git_init_colocated_dirty_working_copy() { Ignored, ), }, + GitStatus { + path: ".jj/workspace-config.binpb", + status: Worktree( + Ignored, + ), + }, GitStatus { path: "new-staged-file", status: Index( @@ -778,7 +784,7 @@ fn test_git_init_colocated_dirty_working_copy() { ), }, ] - "#); + "###); } #[test] diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs index 5df724ac5a4..d9d86d5810b 100644 --- a/cli/tests/test_global_opts.rs +++ b/cli/tests/test_global_opts.rs @@ -492,7 +492,9 @@ fn test_color_config() { "); // Test that per-repo config overrides the user config. - work_dir.write_file(".jj/repo/config.toml", r#"ui.color = "never""#); + work_dir + .run_jj(["config", "set", "--repo", "ui.color", r#""never""#]) + .success(); let output = work_dir.run_jj(["log", "-T", "commit_id"]); insta::assert_snapshot!(output, @r" @ e8849ae12c709f2321908879bc724fdb2ab8a781 diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs index b24aebba382..e62b457b44e 100644 --- a/cli/tests/test_log_command.rs +++ b/cli/tests/test_log_command.rs @@ -1112,7 +1112,9 @@ fn test_default_revset_per_repo() { work_dir.run_jj(["describe", "-m", "add a file"]).success(); // Set configuration to only show the root commit. - work_dir.write_file(".jj/repo/config.toml", r#"revsets.log = "root()""#); + work_dir + .run_jj(["config", "set", "--repo", "revsets.log", r#""root()""#]) + .success(); // Log should only contain one line (for the root commit), and not show the // commit created above. diff --git a/docs/config.md b/docs/config.md index 4952979d771..d2d553cce06 100644 --- a/docs/config.md +++ b/docs/config.md @@ -14,11 +14,9 @@ These are the config settings available to jj/Jujutsu. settings are located in [the user config files], which can be found with `jj config path --user`. -- The repo settings. These can be edited with `jj config edit --repo` and are -located in `.jj/repo/config.toml`. +- The repo settings. These can be edited with `jj config edit --repo`. -- The workspace settings. These can be edited with `jj config edit --workspace` -and are located in `.jj/workspace-config.toml` in the workspace root. +- The workspace settings. These can be edited with `jj config edit --workspace`. - Settings [specified in the command-line](#specifying-config-on-the-command-line). diff --git a/docs/gerrit.md b/docs/gerrit.md index 03453a5e5d4..3c11f243698 100644 --- a/docs/gerrit.md +++ b/docs/gerrit.md @@ -30,12 +30,11 @@ $ jj git clone https://review.gerrithub.io/your/project ``` If you used option 2 You can configure default values in your repository config -by appending the below to `.jj/repo/config.toml`, like so: +by appending the below to your repo config, like so: -```toml -[gerrit] -default-remote = "gerrit" # name of the Git remote to push to -default-remote-branch = "main" # target branch in Gerrit +```shell +$ jj config set gerrit.default-remote +$ jj config set gerrit.default-remote-branch ``` ## Basic workflow diff --git a/docs/github.md b/docs/github.md index 2ee968e8f08..fcf4e33c5ad 100644 --- a/docs/github.md +++ b/docs/github.md @@ -262,8 +262,7 @@ or `master@upstream`. You might want to `jj git fetch` from "upstream" and to `jj git push` to "origin". You can configure the default remotes to fetch from and -push to in your configuration file (for example, -`.jj/repo/config.toml`): +push to in your configuration file (`jj config edit`): ```toml [git] diff --git a/lib/Cargo.toml b/lib/Cargo.toml index ba3b2cfed63..563ec379805 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -40,6 +40,8 @@ chrono = { workspace = true } clru = { workspace = true } digest = { workspace = true } dunce = { workspace = true } +ed25519-dalek = { workspace = true } +etcetera = { workspace = true } either = { workspace = true } futures = { workspace = true } gix = { workspace = true, optional = true } @@ -54,6 +56,7 @@ maplit = { workspace = true } once_cell = { workspace = true } pest = { workspace = true } pest_derive = { workspace = true } +pkcs8 = { workspace = true } pollster = { workspace = true } prost = { workspace = true } rand = { workspace = true } diff --git a/lib/gen-protos/src/main.rs b/lib/gen-protos/src/main.rs index 22c2740b624..3b2e68ac4d5 100644 --- a/lib/gen-protos/src/main.rs +++ b/lib/gen-protos/src/main.rs @@ -22,6 +22,7 @@ fn main() -> Result<()> { "local_working_copy.proto", "simple_op_store.proto", "simple_store.proto", + "user_config.proto", ]; let root = Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap(); diff --git a/lib/src/config.rs b/lib/src/config.rs index 07d510ba127..ea5f9da9740 100644 --- a/lib/src/config.rs +++ b/lib/src/config.rs @@ -589,9 +589,14 @@ impl ConfigFile { } } + /// Retrieves serialized data. + pub fn text(&self) -> String { + self.layer.data.to_string() + } + /// Writes serialized data to the source file. pub fn save(&self) -> Result<(), ConfigFileSaveError> { - fs::write(self.path(), self.layer.data.to_string()) + fs::write(self.path(), self.text()) .context(self.path()) .map_err(ConfigFileSaveError) } diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 521284fee7f..2a12c640ead 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -102,6 +102,7 @@ pub mod secret_backend; pub mod settings; pub mod signing; pub mod tree_merge; +pub mod user_config; // TODO: This file is mostly used for testing, whenever we no longer require it // in the lib it should be moved to the examples (e.g // "examples/simple-backend/"). diff --git a/lib/src/protos/mod.rs b/lib/src/protos/mod.rs index 9bb5313d4d8..3ddc94cdecb 100644 --- a/lib/src/protos/mod.rs +++ b/lib/src/protos/mod.rs @@ -14,3 +14,6 @@ pub mod simple_op_store { pub mod simple_store { include!("simple_store.rs"); } +pub mod user_config { + include!("user_config.rs"); +} diff --git a/lib/src/protos/user_config.proto b/lib/src/protos/user_config.proto new file mode 100644 index 00000000000..e4a8471d3ff --- /dev/null +++ b/lib/src/protos/user_config.proto @@ -0,0 +1,51 @@ +// Copyright 2025 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +package user_config; + +// SecureUserConfig adds cryptographic signatures to ensure that any user +// configuration was generated by the current user. +// See https://github.com/jj-vcs/jj/issues/1595 for more details. +message SecureUserConfig { + // Serialized RepoConfig / WorkspaceConfig + // don't just put it as a submessage here because protobuf serialization is not canonical. + bytes storage = 1; + + // This *probably* isn't necessary, but it doesn't hurt. + bytes salt = 2; + + // If I zip up a repository and send it to you, a valid signature is then + // known to all attackers. They can then send me a zip file including the + // same signature, and I would then have user configs enabled, despite the + // repo coming from someone else. + // We mitigate this risk by putting the path in as part of the signature. + string path = 3; + + // sign(storage + salt + path) + bytes signature = 4; +} + +message RepoConfig { + // The config file for the repo. + // Edit with `jj config edit --repo` + string config = 1; +} + +message WorkspaceConfig { + // The config file for the workspace. + // Edit with `jj config edit --workspace`. + string config = 1; +} diff --git a/lib/src/protos/user_config.rs b/lib/src/protos/user_config.rs new file mode 100644 index 00000000000..221d62c5d0c --- /dev/null +++ b/lib/src/protos/user_config.rs @@ -0,0 +1,38 @@ +// This file is @generated by prost-build. +/// SecureUserConfig adds cryptographic signatures to ensure that any user +/// configuration was generated by the current user. +/// See for more details. +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct SecureUserConfig { + /// Serialized RepoConfig / WorkspaceConfig + /// don't just put it as a submessage here because protobuf serialization is not canonical. + #[prost(bytes = "vec", tag = "1")] + pub storage: ::prost::alloc::vec::Vec, + /// This *probably* isn't necessary, but it doesn't hurt. + #[prost(bytes = "vec", tag = "2")] + pub salt: ::prost::alloc::vec::Vec, + /// If I zip up a repository and send it to you, a valid signature is then + /// known to all attackers. They can then send me a zip file including the + /// same signature, and I would then have user configs enabled, despite the + /// repo coming from someone else. + /// We mitigate this risk by putting the path in as part of the signature. + #[prost(string, tag = "3")] + pub path: ::prost::alloc::string::String, + /// sign(storage + salt + path) + #[prost(bytes = "vec", tag = "4")] + pub signature: ::prost::alloc::vec::Vec, +} +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct RepoConfig { + /// The config file for the repo. + /// Edit with `jj config edit --repo` + #[prost(string, tag = "1")] + pub config: ::prost::alloc::string::String, +} +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct WorkspaceConfig { + /// The config file for the workspace. + /// Edit with `jj config edit --workspace`. + #[prost(string, tag = "1")] + pub config: ::prost::alloc::string::String, +} diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 70e56bf07d3..7dab704e739 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -26,6 +26,7 @@ use std::slice; use std::sync::Arc; use itertools::Itertools as _; +use jj_lib::user_config::write_user_config; use once_cell::sync::OnceCell; use pollster::FutureExt as _; use thiserror::Error; @@ -75,6 +76,7 @@ use crate::op_store::RemoteRef; use crate::op_store::RemoteRefState; use crate::op_store::RootOperationData; use crate::operation::Operation; +use crate::protos::user_config::RepoConfig; use crate::ref_name::GitRefName; use crate::ref_name::RefName; use crate::ref_name::RemoteName; @@ -107,6 +109,7 @@ use crate::submodule_store::SubmoduleStore; use crate::transaction::Transaction; use crate::transaction::TransactionCommitError; use crate::tree_merge::MergeOptions; +use crate::user_config::UserConfigError; use crate::view::RenameWorkspaceError; use crate::view::View; @@ -165,6 +168,8 @@ pub enum RepoInitError { OpHeadsStore(#[from] OpHeadsStoreError), #[error(transparent)] Path(#[from] PathError), + #[error(transparent)] + UserConfigError(#[from] UserConfigError), } impl ReadonlyRepo { @@ -201,6 +206,7 @@ impl ReadonlyRepo { let store_path = repo_path.join("store"); fs::create_dir(&store_path).context(&store_path)?; + write_user_config(&repo_path, &RepoConfig::default())?; let backend = backend_initializer(settings, &store_path)?; let backend_path = store_path.join("type"); fs::write(&backend_path, backend.name()).context(&backend_path)?; diff --git a/lib/src/user_config.rs b/lib/src/user_config.rs new file mode 100644 index 00000000000..82d4ce524ed --- /dev/null +++ b/lib/src/user_config.rs @@ -0,0 +1,251 @@ +// Copyright 2025 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! A wrapper around retrieval and storage of secure user configuration. + +use std::fmt::Debug; +use std::fs; +use std::io; +use io::Write as _; +use std::path::Path; + +use ed25519_dalek::Signer as _; +use ed25519_dalek::SigningKey; +use etcetera::BaseStrategy as _; +use pkcs8::DecodePrivateKey as _; +use pkcs8::EncodePrivateKey as _; +use pkcs8::EncodePublicKey as _; +use prost::Message as _; +use rand::RngCore as _; +use rand::TryRngCore as _; +use rand::rngs::OsRng; +use tempfile::NamedTempFile; +use thiserror::Error; + +use crate::file_util::IoResultExt as _; +use crate::file_util::PathError; +use crate::protos::user_config::RepoConfig; +use crate::protos::user_config::SecureUserConfig; +use crate::protos::user_config::WorkspaceConfig; + +#[derive(Error, Debug)] +/// Error occurred while dealing with user configs +pub enum UserConfigError { + /// Failed to determine the home directory + #[error("Unable to determine the home directory.")] + HomeDirNotFound, + /// Failed to read / write to the specified path + #[error(transparent)] + PathError(#[from] PathError), + /// Failed to load the private key. + #[error(transparent)] + PrivateKeyError(#[from] ed25519_dalek::pkcs8::Error), + /// Failed to load the public key. + #[error(transparent)] + PublicKeyError(#[from] ed25519_dalek::pkcs8::spki::Error), + + /// Failed to decoode the user configuration file. + #[error(transparent)] + DecodeError(#[from] prost::DecodeError), + + /// Missing the user configuration file. + #[error( + "This repo was generated by an old version of jj and does not have secure user \ + configuration" + )] + LegacyRepo, +} + +/// A config file represented by a string instead of backed by a file. +pub trait ConfigType: prost::Message + Default + Clone + Debug { + /// The name of this type. + fn kind() -> &'static str; + /// The filename of the configuration file to write. + fn filename() -> &'static str; + /// The string containing the config. + fn config(&self) -> &str; +} + +impl ConfigType for RepoConfig { + fn kind() -> &'static str { + "repo" + } + + fn filename() -> &'static str { + "config.binpb" + } + + fn config(&self) -> &str { + &self.config + } +} + +impl ConfigType for WorkspaceConfig { + fn kind() -> &'static str { + "workspace" + } + + fn filename() -> &'static str { + "workspace-config.binpb" + } + + fn config(&self) -> &str { + &self.config + } +} + +/// The result of a read of user configuration. +#[derive(Clone, Debug)] +pub enum UserConfig { + /// We had the correct signature. + Trusted(T), + /// Repository has moved from one location to another. + RepoMoved { + /// The old path to the repo, encoded as a string. + from: String, + /// The new path to the repo, encoded as a string. + to: String, + /// The configuration that the repo tried to use. + config: T, + }, + + /// Repository had the wrong signature. + InvalidSignature(T), +} + +impl UserConfig { + /// Retrieves the underlying config, regardless of the level of trust. + pub fn get(&self) -> &T { + match self { + Self::Trusted(config) + | Self::RepoMoved { config, .. } + | Self::InvalidSignature(config) => config, + } + } +} + +/// Returns a signing key associated with the current user. +/// Generates a key if we don't yet have one. +fn signing_key() -> Result { + let strategy = match etcetera::choose_base_strategy() { + Ok(s) => s, + Err(_) => return Err(UserConfigError::HomeDirNotFound), + }; + let key_dir = strategy.data_dir().join("jj/config/keys"); + let signing_key = key_dir.join("ed25519.der"); + let verifying_key = key_dir.join("ed25519.der.pub"); + Ok(match fs::read(&signing_key).context(&signing_key) { + Ok(bytes) => SigningKey::from_pkcs8_der(&bytes)?, + Err(e) if e.source.kind() == io::ErrorKind::NotFound => { + fs::create_dir_all(&key_dir).context(&key_dir)?; + + let mut csprng = OsRng.unwrap_err(); + let key = SigningKey::generate(&mut csprng); + // We never actually use the public key. + // But write it to disk so we don't lose it in case we decide to use it in the + // future. + key.verifying_key() + .write_public_key_der_file(&verifying_key)?; + key.write_pkcs8_der_file(&signing_key)?; + key + } + Err(e) => return Err(UserConfigError::from(e)), + }) +} + +/// Reads the user configuration from a repository for the current user. +pub fn read_user_config(dir: &Path) -> Result, UserConfigError> { + let path = dir.join(T::filename()); + let buf = match fs::read(&path).context(&path) { + Ok(buf) => buf, + Err(e) if e.source.kind() == io::ErrorKind::NotFound => { + if dir.exists() { + return Err(UserConfigError::LegacyRepo); + } else { + // This repo doesn't exist. So we create some new empty config. + return Ok(UserConfig::Trusted(Default::default())); + } + } + Err(e) => return Err(UserConfigError::PathError(e)), + }; + let secure_config = SecureUserConfig::decode(&*buf)?; + let config = T::decode(&*secure_config.storage)?; + let key = match signing_key() { + Ok(key) => key, + // If we're unable to determine the signing key, we unfortunately can't + // really do anything but trust it. + Err(_) => return Ok(UserConfig::Trusted(config)), + }; + + let sign = |s: &str| { + let mut to_sign = secure_config.storage.clone(); + to_sign.extend(&secure_config.salt); + to_sign.extend(s.as_bytes()); + key.sign(&to_sign).to_bytes() + }; + + let canonical = dunce::canonicalize(dir) + .context(dir) + .map_err(UserConfigError::PathError)? + .to_string_lossy() + .to_string(); + if sign(&canonical) == *secure_config.signature { + Ok(UserConfig::Trusted(config)) + } else if config.config().is_empty() { + // We don't trust the providence of the configuration, but it's empty so we'll + // just replace it with a similarly empty one. + Ok(UserConfig::Trusted(Default::default())) + } else if sign(&secure_config.path) == *secure_config.signature { + Ok(UserConfig::RepoMoved { + from: secure_config.path, + to: canonical, + config, + }) + } else { + Ok(UserConfig::InvalidSignature(config)) + } +} + +/// Writes a given configuration for a repository or workspace. +pub fn write_user_config(dir: &Path, config: &T) -> Result<(), UserConfigError> { + let content = config.encode_to_vec(); + let mut salt = [0u8; 8]; + rand::rng().fill_bytes(&mut salt); + let canonical = dunce::canonicalize(dir) + .context(dir)? + .to_string_lossy() + .to_string(); + + let signature = signing_key().ok().map(|key| { + let mut to_sign = content.clone(); + to_sign.extend(salt); + to_sign.extend(canonical.as_bytes()); + key.sign(&to_sign).to_vec() + }); + + let secure_config = SecureUserConfig { + storage: content, + salt: salt.to_vec(), + path: canonical, + signature: signature.unwrap_or_default(), + }; + + // Atomically write to the file. + let mut f = NamedTempFile::new_in(dir).context(dir)?; + f.write_all(&secure_config.encode_to_vec()).context(dir)?; + let out = dir.join(T::filename()); + fs::rename(f.path(), &out).context(&out)?; + + Ok(()) +} diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index ae19d14b3e7..c09f9165669 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -35,6 +35,7 @@ use crate::local_working_copy::LocalWorkingCopy; use crate::local_working_copy::LocalWorkingCopyFactory; use crate::op_heads_store::OpHeadsStoreError; use crate::op_store::OperationId; +use crate::protos::user_config::WorkspaceConfig; use crate::ref_name::WorkspaceName; use crate::ref_name::WorkspaceNameBuf; use crate::repo::BackendInitializer; @@ -55,6 +56,8 @@ use crate::signing::SignInitError; use crate::signing::Signer; use crate::simple_backend::SimpleBackend; use crate::transaction::TransactionCommitError; +use crate::user_config::UserConfigError; +use crate::user_config::write_user_config; use crate::working_copy::CheckoutError; use crate::working_copy::CheckoutStats; use crate::working_copy::LockedWorkingCopy; @@ -82,6 +85,8 @@ pub enum WorkspaceInitError { SignInit(#[from] SignInitError), #[error(transparent)] TransactionCommit(#[from] TransactionCommitError), + #[error(transparent)] + UserConfigError(#[from] UserConfigError), } #[derive(Error, Debug)] @@ -118,7 +123,10 @@ pub struct Workspace { fn create_jj_dir(workspace_root: &Path) -> Result { let jj_dir = workspace_root.join(".jj"); match std::fs::create_dir(&jj_dir).context(&jj_dir) { - Ok(()) => Ok(jj_dir), + Ok(()) => { + write_user_config(&jj_dir, &WorkspaceConfig::default())?; + Ok(jj_dir) + } Err(ref e) if e.source.kind() == io::ErrorKind::AlreadyExists => { Err(WorkspaceInitError::DestinationExists(jj_dir)) } @@ -311,6 +319,7 @@ impl Workspace { RepoInitError::Backend(err) => WorkspaceInitError::Backend(err), RepoInitError::OpHeadsStore(err) => WorkspaceInitError::OpHeadsStore(err), RepoInitError::Path(err) => WorkspaceInitError::Path(err), + RepoInitError::UserConfigError(err) => WorkspaceInitError::UserConfigError(err), })?; let (working_copy, repo) = init_working_copy( &repo,