From 03af58e2b69919bf7d2e5238cff152dd36ed9046 Mon Sep 17 00:00:00 2001 From: Matt Stark Date: Thu, 4 Dec 2025 10:39:45 +1100 Subject: [PATCH] config: Implement secure-config design doc. See docs/design/secure-config.md for many more details. Fixes #3303 Fixes #1595 --- CHANGELOG.md | 5 + Cargo.lock | 2 + cli/Cargo.toml | 2 + cli/examples/custom-backend/main.rs | 4 +- cli/examples/custom-working-copy/main.rs | 4 +- cli/src/cli_util.rs | 19 +- cli/src/command_error.rs | 7 + cli/src/commands/config/mod.rs | 17 +- cli/src/commands/config/path.rs | 8 +- cli/src/commands/debug/init_simple.rs | 5 +- cli/src/commands/git/clone.rs | 16 +- cli/src/commands/git/init.rs | 17 +- cli/src/commands/git/mod.rs | 12 +- cli/src/complete.rs | 12 +- cli/src/config.rs | 176 ++++--- cli/tests/cli-reference@.md.snap | 2 + cli/tests/common/test_environment.rs | 10 + cli/tests/test_alias.rs | 13 +- cli/tests/test_config_command.rs | 80 +-- cli/tests/test_global_opts.rs | 5 +- cli/tests/test_log_command.rs | 6 +- docs/config.md | 10 +- docs/design/secure-config.md | 2 + docs/gerrit.md | 5 +- docs/github.md | 4 +- lib/gen-protos/src/main.rs | 1 + lib/src/lib.rs | 1 + lib/src/protos/mod.rs | 3 + lib/src/protos/secure_config.proto | 25 + lib/src/protos/secure_config.rs | 10 + lib/src/secure_config.rs | 590 +++++++++++++++++++++++ 31 files changed, 929 insertions(+), 144 deletions(-) create mode 100644 lib/src/protos/secure_config.proto create mode 100644 lib/src/protos/secure_config.rs create mode 100644 lib/src/secure_config.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 87b1e4b8d..95fe5f107 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). conflict came from (currently this is only supported for conflicts created by certain commands). +* Per-repo and per-workspace config is now stored outside the repo, for security + reasons. This is not a breaking change because we automatically migrate + legacy repos to this new format. `.jj/repo/config.toml` and + `.jj/workspace-config.toml` should no longer be used. + ### Fixed bugs * Broken symlink on Windows. [#6934](https://github.com/jj-vcs/jj/issues/6934). diff --git a/Cargo.lock b/Cargo.lock index 025c28bea..de40916e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2382,6 +2382,8 @@ dependencies = [ "pollster", "proptest", "proptest-state-machine", + "rand 0.9.2", + "rand_chacha", "rayon", "regex", "rpassword", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index d59ee3109..8b55defa2 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -82,6 +82,8 @@ once_cell = { workspace = true } pest = { workspace = true } pest_derive = { workspace = true } pollster = { workspace = true } +rand = { workspace = true } +rand_chacha = { 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 bae68d25c..28181a684 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)?.0; // 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 cadf2aae6..af06a257c 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)?.0; 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 b494b28a9..c734bb1ec 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -354,7 +354,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 } @@ -362,19 +362,20 @@ 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 { + ) -> Result<(UserSettings, ConfigEnv), CommandError> { 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.reload_repo_config(&mut raw_config)?; + config_env.reload_repo_config(ui, &mut raw_config)?; config_env.reset_workspace_path(workspace_root); - config_env.reload_workspace_config(&mut raw_config)?; + config_env.reload_workspace_config(ui, &mut raw_config)?; let mut config = config_env.resolve_config(&raw_config)?; // No migration messages here, which would usually be emitted before. jj_lib::config::migrate(&mut config, &self.data.config_migrations)?; - Ok(self.data.settings.with_new_config(config)?) + Ok((self.data.settings.with_new_config(config)?, config_env)) } /// Loads text editor from the settings. @@ -3965,9 +3966,9 @@ impl<'a> CliRunner<'a> { 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.reload_repo_config(&mut raw_config)?; + config_env.reload_repo_config(ui, &mut raw_config)?; config_env.reset_workspace_path(loader.workspace_root()); - config_env.reload_workspace_config(&mut raw_config)?; + config_env.reload_workspace_config(ui, &mut raw_config)?; } let mut config = config_env.resolve_config(&raw_config)?; migrate_config(&mut config)?; @@ -4011,9 +4012,9 @@ impl<'a> CliRunner<'a> { .create(&abs_path) .map_err(|err| map_workspace_load_error(err, Some(path)))?; config_env.reset_repo_path(loader.repo_path()); - config_env.reload_repo_config(&mut raw_config)?; + config_env.reload_repo_config(ui, &mut raw_config)?; config_env.reset_workspace_path(loader.workspace_root()); - config_env.reload_workspace_config(&mut raw_config)?; + config_env.reload_workspace_config(ui, &mut raw_config)?; Ok(loader) } else { maybe_cwd_workspace_loader diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 962ce2c47..ea7237cf3 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -52,6 +52,7 @@ use jj_lib::revset::RevsetEvaluationError; use jj_lib::revset::RevsetParseError; use jj_lib::revset::RevsetParseErrorKind; use jj_lib::revset::RevsetResolutionError; +use jj_lib::secure_config::SecureConfigError; use jj_lib::str_util::StringPatternParseError; use jj_lib::trailer::TrailerParseError; use jj_lib::transaction::TransactionCommitError; @@ -766,6 +767,12 @@ impl From for CommandError { } } +impl From for CommandError { + fn from(err: SecureConfigError) -> Self { + internal_error_with_message("Failed to determine the secure config for a repo", err) + } +} + fn find_source_parse_error_hint(err: &dyn error::Error) -> Option { let source = err.source()?; if let Some(source) = source.downcast_ref() { diff --git a/cli/src/commands/config/mod.rs b/cli/src/commands/config/mod.rs index 7a1631f12..9bf93b90d 100644 --- a/cli/src/commands/config/mod.rs +++ b/cli/src/commands/config/mod.rs @@ -19,7 +19,7 @@ mod path; mod set; mod unset; -use std::path::Path; +use std::path::PathBuf; use itertools::Itertools as _; use jj_lib::config::ConfigFile; @@ -73,21 +73,24 @@ impl ConfigLevelArgs { } } - fn config_paths<'a>(&self, config_env: &'a ConfigEnv) -> Result, CommandError> { + fn config_paths(&self, ui: &Ui, config_env: &ConfigEnv) -> Result, CommandError> { if self.user { - let paths = config_env.user_config_paths().collect_vec(); + let paths = config_env + .user_config_paths() + .map(|p| p.to_path_buf()) + .collect_vec(); if paths.is_empty() { return Err(user_error("No user config path found")); } Ok(paths) } else if self.repo { config_env - .repo_config_path() + .repo_config_path(ui)? .map(|p| vec![p]) .ok_or_else(|| user_error("No repo config path found")) } else if self.workspace { config_env - .workspace_config_path() + .workspace_config_path(ui)? .map(|p| vec![p]) .ok_or_else(|| user_error("No workspace config path found")) } else { @@ -124,12 +127,12 @@ impl ConfigLevelArgs { ) } else if self.repo { pick_one( - config_env.repo_config_files(config)?, + config_env.repo_config_files(config, ui)?, "No repo config path found to edit", ) } else if self.workspace { pick_one( - config_env.workspace_config_files(config)?, + config_env.workspace_config_files(config, ui)?, "No workspace config path found to edit", ) } else { diff --git a/cli/src/commands/config/path.rs b/cli/src/commands/config/path.rs index df5bd40ce..edb657c77 100644 --- a/cli/src/commands/config/path.rs +++ b/cli/src/commands/config/path.rs @@ -27,6 +27,10 @@ use crate::ui::Ui; /// /// A config file at that path may or may not exist. /// +/// If `--repo` or `--workspace` is specified and the config file does not +/// exist, jj will generate a new config directory for this repo/workspace and +/// print the path to the config file in that directory. +/// /// See `jj config edit` if you'd like to immediately edit a file. #[derive(clap::Args, Clone, Debug)] pub struct ConfigPathArgs { @@ -40,8 +44,8 @@ pub fn cmd_config_path( command: &CommandHelper, args: &ConfigPathArgs, ) -> Result<(), CommandError> { - for config_path in args.level.config_paths(command.config_env())? { - let path_bytes = file_util::path_to_bytes(config_path).map_err(user_error)?; + for config_path in args.level.config_paths(ui, command.config_env())? { + let path_bytes = file_util::path_to_bytes(&config_path).map_err(user_error)?; ui.stdout().write_all(path_bytes)?; writeln!(ui.stdout())?; } diff --git a/cli/src/commands/debug/init_simple.rs b/cli/src/commands/debug/init_simple.rs index a2a8fe967..6a1186221 100644 --- a/cli/src/commands/debug/init_simple.rs +++ b/cli/src/commands/debug/init_simple.rs @@ -57,7 +57,10 @@ 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)?.0, + &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 94ac3ec17..942bda3fa 100644 --- a/cli/src/commands/git/clone.rs +++ b/cli/src/commands/git/clone.rs @@ -42,6 +42,7 @@ use crate::command_error::user_error; use crate::command_error::user_error_with_message; use crate::commands::git::FetchTagsMode; use crate::commands::git::maybe_add_gitignore; +use crate::config::ConfigEnv; use crate::git_util::absolute_git_url; use crate::git_util::load_git_import_options; use crate::git_util::print_git_import_stats; @@ -176,7 +177,8 @@ pub fn cmd_git_clone( .map_err(|err| user_error_with_message(format!("Failed to create {wc_path_str}"), err))?; let clone_result = (|| -> Result<_, CommandError> { - let workspace_command = init_workspace(ui, command, &canonical_wc_path, colocate)?; + let (workspace_command, config_env) = + init_workspace(ui, command, &canonical_wc_path, colocate)?; let mut workspace_command = configure_remote( ui, command, @@ -198,7 +200,7 @@ pub fn cmd_git_clone( args.depth, args.fetch_tags, )?; - Ok((workspace_command, default_branch)) + Ok((workspace_command, default_branch, config_env)) })(); if clone_result.is_err() { let clean_up_dirs = || -> io::Result<()> { @@ -226,12 +228,12 @@ pub fn cmd_git_clone( } } - let (mut workspace_command, (working_branch, working_is_default)) = clone_result?; + let (mut workspace_command, (working_branch, working_is_default), config_env) = clone_result?; if let Some(name) = &working_branch { let working_symbol = name.to_remote_symbol(remote_name); if working_is_default { - write_repository_level_trunk_alias(ui, workspace_command.repo_path(), working_symbol)?; + write_repository_level_trunk_alias(ui, &config_env, working_symbol)?; } let working_branch_remote_ref = workspace_command .repo() @@ -264,8 +266,8 @@ fn init_workspace( command: &CommandHelper, wc_path: &Path, colocate: bool, -) -> Result { - let settings = command.settings_for_new_workspace(wc_path)?; +) -> Result<(WorkspaceCommandHelper, ConfigEnv), CommandError> { + let (settings, config_env) = command.settings_for_new_workspace(ui, wc_path)?; let (workspace, repo) = if colocate { Workspace::init_colocated_git(&settings, wc_path)? } else { @@ -273,7 +275,7 @@ fn init_workspace( }; let workspace_command = command.for_workable_repo(ui, workspace, repo)?; maybe_add_gitignore(&workspace_command)?; - Ok(workspace_command) + Ok((workspace_command, config_env)) } fn configure_remote( diff --git a/cli/src/commands/git/init.rs b/cli/src/commands/git/init.rs index 5769da847..80542992c 100644 --- a/cli/src/commands/git/init.rs +++ b/cli/src/commands/git/init.rs @@ -31,7 +31,6 @@ use jj_lib::workspace::Workspace; use super::write_repository_level_trunk_alias; use crate::cli_util::CommandHelper; -use crate::cli_util::WorkspaceCommandHelper; use crate::cli_util::start_repo_transaction; use crate::command_error::CommandError; use crate::command_error::cli_error; @@ -39,6 +38,7 @@ use crate::command_error::internal_error; use crate::command_error::user_error_with_hint; use crate::command_error::user_error_with_message; use crate::commands::git::maybe_add_gitignore; +use crate::config::ConfigEnv; use crate::formatter::FormatterExt as _; use crate::git_util::is_colocated_git_workspace; use crate::git_util::load_git_import_options; @@ -185,7 +185,7 @@ fn do_init( GitInitMode::Internal }; - let settings = command.settings_for_new_workspace(workspace_root)?; + let (settings, config_env) = command.settings_for_new_workspace(ui, workspace_root)?; match &init_mode { GitInitMode::Colocate => { let (workspace, repo) = Workspace::init_colocated_git(&settings, workspace_root)?; @@ -202,7 +202,11 @@ fn do_init( let mut workspace_command = command.for_workable_repo(ui, workspace, repo)?; maybe_add_gitignore(&workspace_command)?; workspace_command.maybe_snapshot(ui)?; - maybe_set_repository_level_trunk_alias(ui, &workspace_command)?; + maybe_set_repository_level_trunk_alias( + ui, + &git::get_git_repo(workspace_command.repo().store())?, + &config_env, + )?; if !workspace_command.working_copy_shared_with_git() { let mut tx = workspace_command.start_transaction(); jj_lib::git::import_head(tx.repo_mut())?; @@ -264,10 +268,9 @@ fn init_git_refs( // Checks "upstream" first, then "origin" as fallback. pub fn maybe_set_repository_level_trunk_alias( ui: &Ui, - workspace_command: &WorkspaceCommandHelper, + git_repo: &gix::Repository, + config_env: &ConfigEnv, ) -> Result<(), CommandError> { - let git_repo = git::get_git_repo(workspace_command.repo().store())?; - // Try "upstream" first, then fall back to "origin" for remote in ["upstream", "origin"] { let ref_name = format!("refs/remotes/{remote}/HEAD"); @@ -287,7 +290,7 @@ pub fn maybe_set_repository_level_trunk_alias( { // TODO: Can we assume the symbolic target points to the same remote? let symbol = symbol.name.to_remote_symbol(remote.as_ref()); - write_repository_level_trunk_alias(ui, workspace_command.repo_path(), symbol)?; + write_repository_level_trunk_alias(ui, config_env, symbol)?; } return Ok(()); } diff --git a/cli/src/commands/git/mod.rs b/cli/src/commands/git/mod.rs index 908f83ec2..78b91dd9c 100644 --- a/cli/src/commands/git/mod.rs +++ b/cli/src/commands/git/mod.rs @@ -23,7 +23,6 @@ mod remote; mod root; use std::io::Write as _; -use std::path::Path; use clap::Subcommand; use clap::ValueEnum; @@ -58,6 +57,7 @@ use crate::cli_util::CommandHelper; use crate::cli_util::WorkspaceCommandHelper; use crate::command_error::CommandError; use crate::command_error::user_error_with_message; +use crate::config::ConfigEnv; use crate::ui::Ui; /// Commands for working with Git remotes and the underlying Git repo @@ -128,10 +128,16 @@ fn get_single_remote(store: &Store) -> Result, UnexpectedG /// Sets repository level `trunk()` alias to the specified remote symbol. fn write_repository_level_trunk_alias( ui: &Ui, - repo_path: &Path, + config_env: &ConfigEnv, symbol: RemoteRefSymbol<'_>, ) -> Result<(), CommandError> { - let mut file = ConfigFile::load_or_empty(ConfigSource::Repo, repo_path.join("config.toml"))?; + let config_path = if let Some(path) = config_env.repo_config_path(ui)? { + path + } else { + // We couldn't find the user's home directory, so we skip this step. + return Ok(()); + }; + let mut file = ConfigFile::load_or_empty(ConfigSource::Repo, config_path)?; file.set_value(["revset-aliases", "trunk()"], symbol.to_string()) .expect("initial repo config shouldn't have invalid values"); file.save()?; diff --git a/cli/src/complete.rs b/cli/src/complete.rs index 1d457087f..752c1b369 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -1081,9 +1081,11 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { config_env.reload_user_config(&mut raw_config).ok(); if let Ok(loader) = &maybe_cwd_workspace_loader { config_env.reset_repo_path(loader.repo_path()); - config_env.reload_repo_config(&mut raw_config).ok(); + config_env.reload_repo_config(&ui, &mut raw_config).ok(); config_env.reset_workspace_path(loader.workspace_root()); - config_env.reload_workspace_config(&mut raw_config).ok(); + config_env + .reload_workspace_config(&ui, &mut raw_config) + .ok(); } let mut config = config_env.resolve_config(&raw_config)?; // skip 2 because of the clap_complete prelude: jj -- jj @@ -1101,9 +1103,11 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { // 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.reload_repo_config(&mut raw_config).ok(); + config_env.reload_repo_config(&ui, &mut raw_config).ok(); config_env.reset_workspace_path(loader.workspace_root()); - config_env.reload_workspace_config(&mut raw_config).ok(); + config_env + .reload_workspace_config(&ui, &mut raw_config) + .ok(); 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 e7bb3f8d0..62a43fe1b 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -21,7 +21,9 @@ use std::fmt; use std::path::Path; use std::path::PathBuf; use std::process::Command; +use std::sync::Arc; use std::sync::LazyLock; +use std::sync::Mutex; use etcetera::BaseStrategy as _; use itertools::Itertools as _; @@ -35,6 +37,10 @@ use jj_lib::config::ConfigResolutionContext; use jj_lib::config::ConfigSource; use jj_lib::config::ConfigValue; use jj_lib::config::StackedConfig; +use jj_lib::secure_config::LoadedSecureConfig; +use jj_lib::secure_config::SecureConfig; +use rand::SeedableRng as _; +use rand_chacha::ChaCha20Rng; use regex::Captures; use regex::Regex; use serde::Serialize as _; @@ -43,10 +49,14 @@ use tracing::instrument; use crate::command_error::CommandError; use crate::command_error::config_error; use crate::command_error::config_error_with_message; +use crate::ui::Ui; // TODO(#879): Consider generating entire schema dynamically vs. static file. pub const CONFIG_SCHEMA: &str = include_str!("config-schema.json"); +pub const REPO_CONFIG_DIR: &str = "repos"; +pub const WORKSPACE_CONFIG_DIR: &str = "workspaces"; + /// Parses a TOML value expression. Interprets the given value as string if it /// can't be parsed and doesn't look like a TOML expression. pub fn parse_value_or_bare_string(value_str: &str) -> Result { @@ -269,6 +279,10 @@ struct UnresolvedConfigEnv { } impl UnresolvedConfigEnv { + fn root_config_dir(&self) -> Option { + self.config_dir.as_deref().map(|c| c.join("jj")) + } + fn resolve(self) -> Vec { if let Some(paths) = self.jj_config { return split_paths(&paths) @@ -318,13 +332,15 @@ impl UnresolvedConfigEnv { #[derive(Clone, Debug)] pub struct ConfigEnv { home_dir: Option, + root_config_dir: Option, repo_path: Option, workspace_path: Option, user_config_paths: Vec, - repo_config_path: Option, - workspace_config_path: Option, + repo_config: Option, + workspace_config: Option, command: Option, hostname: Option, + rng: Arc>, } impl ConfigEnv { @@ -347,13 +363,23 @@ impl ConfigEnv { }; Self { home_dir, + root_config_dir: env.root_config_dir(), repo_path: None, workspace_path: None, user_config_paths: env.resolve(), - repo_config_path: None, - workspace_config_path: None, + repo_config: None, + workspace_config: None, command: None, hostname: whoami::fallible::hostname().ok(), + // We would ideally use JjRng, but that requires the seed from the + // config, which requires the config to be loaded. + rng: Arc::new(Mutex::new( + if let Ok(Ok(value)) = env::var("JJ_RANDOMNESS_SEED").map(|s| s.parse::()) { + ChaCha20Rng::seed_from_u64(value) + } else { + ChaCha20Rng::from_os_rng() + }, + )), } } @@ -381,11 +407,10 @@ impl ConfigEnv { /// The parent directory for the new file may be created by this function. /// If the user configuration path is unknown, this function returns an /// empty `Vec`. - pub fn user_config_files( - &self, - config: &RawConfig, - ) -> Result, ConfigLoadError> { - config_files_for(config, ConfigSource::User, || self.new_user_config_file()) + pub fn user_config_files(&self, config: &RawConfig) -> Result, CommandError> { + config_files_for(config, ConfigSource::User, || { + Ok(self.new_user_config_file()?) + }) } fn new_user_config_file(&self) -> Result, ConfigLoadError> { @@ -419,24 +444,51 @@ impl ConfigEnv { Ok(()) } - /// Sets the directory where repo-specific config file is stored. The path - /// is usually `.jj/repo`. + /// Sets the directory where the repo-specific config file is stored. The + /// path is usually `$REPO/.jj/repo`. pub fn reset_repo_path(&mut self, path: &Path) { + self.repo_config = Some(SecureConfig::new_repo(path.to_path_buf())); self.repo_path = Some(path.to_owned()); - self.repo_config_path = Some(ConfigPath::new(path.join("config.toml"))); } - /// 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()) + fn load_secure_config( + &self, + ui: &Ui, + config: &Option, + kind: &str, + force: bool, + ) -> Result, CommandError> { + Ok(match (config, self.root_config_dir.as_ref()) { + (Some(config), Some(root_config_dir)) => { + let mut guard = self.rng.lock().unwrap(); + let loaded_config = if force { + config.load_config(&mut guard, &root_config_dir.join(kind)) + } else { + config.maybe_load_config(&mut guard, &root_config_dir.join(kind)) + }?; + for warning in &loaded_config.warnings { + writeln!(ui.warning_default(), "{warning}")?; + } + Some(loaded_config) + } + _ => None, + }) } /// 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, - } + fn maybe_repo_config_path(&self, ui: &Ui) -> Result, CommandError> { + Ok(self + .load_secure_config(ui, &self.repo_config, REPO_CONFIG_DIR, false)? + .and_then(|c| c.config_file)) + } + + /// Returns a path to the existing repo-specific config file. + /// If the config file does not exist, will create a new config ID and + /// create a new directory for this. + pub fn repo_config_path(&self, ui: &Ui) -> Result, CommandError> { + Ok(self + .load_secure_config(ui, &self.repo_config, REPO_CONFIG_DIR, true)? + .and_then(|c| c.config_file)) } /// Returns repo configuration files for modification. Instantiates one if @@ -448,49 +500,53 @@ impl ConfigEnv { pub fn repo_config_files( &self, config: &RawConfig, - ) -> Result, ConfigLoadError> { - config_files_for(config, ConfigSource::Repo, || self.new_repo_config_file()) + ui: &Ui, + ) -> Result, CommandError> { + config_files_for(config, ConfigSource::Repo, || self.new_repo_config_file(ui)) } - fn new_repo_config_file(&self) -> Result, ConfigLoadError> { - self.repo_config_path() + fn new_repo_config_file(&self, ui: &Ui) -> Result, CommandError> { + Ok(self + .repo_config_path(ui)? // 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() + .transpose()?) } /// Loads repo-specific config file into the given `config`. The old /// repo-config layer will be replaced if any. - #[instrument] - pub fn reload_repo_config(&self, config: &mut RawConfig) -> Result<(), ConfigLoadError> { + #[instrument(skip(ui))] + pub fn reload_repo_config(&self, ui: &Ui, config: &mut RawConfig) -> Result<(), CommandError> { config.as_mut().remove_layers(ConfigSource::Repo); - if let Some(path) = self.existing_repo_config_path() { + if let Some(path) = self.maybe_repo_config_path(ui)? + && path.exists() + { config.as_mut().load_file(ConfigSource::Repo, path)?; } Ok(()) } - /// Sets the directory for the workspace and the workspace-specific config - /// file. + /// Sets the directory where the workspace-specific config file is stored. pub fn reset_workspace_path(&mut self, path: &Path) { + self.workspace_config = Some(SecureConfig::new_workspace(path.join(".jj"))); self.workspace_path = Some(path.to_owned()); - self.workspace_config_path = Some(ConfigPath::new( - path.join(".jj").join("workspace-config.toml"), - )); } - /// 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()) + /// Returns a path to the workspace-specific config file, if it exists. + fn maybe_workspace_config_path(&self, ui: &Ui) -> Result, CommandError> { + Ok(self + .load_secure_config(ui, &self.workspace_config, WORKSPACE_CONFIG_DIR, false)? + .and_then(|c| c.config_file)) } /// 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, - } + /// If the config file does not exist, will create a new config ID and + /// create a new directory for this. + pub fn workspace_config_path(&self, ui: &Ui) -> Result, CommandError> { + Ok(self + .load_secure_config(ui, &self.workspace_config, WORKSPACE_CONFIG_DIR, true)? + .and_then(|c| c.config_file)) } /// Returns workspace configuration files for modification. Instantiates one @@ -502,24 +558,32 @@ impl ConfigEnv { pub fn workspace_config_files( &self, config: &RawConfig, - ) -> Result, ConfigLoadError> { + ui: &Ui, + ) -> Result, CommandError> { config_files_for(config, ConfigSource::Workspace, || { - self.new_workspace_config_file() + self.new_workspace_config_file(ui) }) } - fn new_workspace_config_file(&self) -> Result, ConfigLoadError> { - self.workspace_config_path() + fn new_workspace_config_file(&self, ui: &Ui) -> Result, CommandError> { + Ok(self + .workspace_config_path(ui)? .map(|path| ConfigFile::load_or_empty(ConfigSource::Workspace, path)) - .transpose() + .transpose()?) } /// Loads workspace-specific config file into the given `config`. The old /// workspace-config layer will be replaced if any. - #[instrument] - pub fn reload_workspace_config(&self, config: &mut RawConfig) -> Result<(), ConfigLoadError> { + #[instrument(skip(ui))] + pub fn reload_workspace_config( + &self, + ui: &Ui, + config: &mut RawConfig, + ) -> Result<(), CommandError> { config.as_mut().remove_layers(ConfigSource::Workspace); - if let Some(path) = self.existing_workspace_config_path() { + if let Some(path) = self.maybe_workspace_config_path(ui)? + && path.exists() + { config.as_mut().load_file(ConfigSource::Workspace, path)?; } Ok(()) @@ -542,8 +606,8 @@ impl ConfigEnv { fn config_files_for( config: &RawConfig, source: ConfigSource, - new_file: impl FnOnce() -> Result, ConfigLoadError>, -) -> Result, ConfigLoadError> { + new_file: impl FnOnce() -> Result, CommandError>, +) -> Result, CommandError> { let mut files = config .as_ref() .layers_for(source) @@ -563,8 +627,8 @@ fn config_files_for( /// 1. Default /// 2. Base environment variables /// 3. [User configs](https://docs.jj-vcs.dev/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` /// @@ -1727,13 +1791,15 @@ mod tests { }; ConfigEnv { home_dir, + root_config_dir: None, repo_path: None, workspace_path: None, user_config_paths: env.resolve(), - repo_config_path: None, - workspace_config_path: None, + repo_config: None, + workspace_config: None, command: None, hostname: None, + rng: Arc::new(Mutex::new(ChaCha20Rng::seed_from_u64(0))), } } } diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index bb84fd5b4..d5e255b77 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -754,6 +754,8 @@ Print the paths to the config files A config file at that path may or may not exist. +If `--repo` or `--workspace` is specified and the config file does not exist, jj will generate a new config directory for this repo/workspace and print the path to the config file in that directory. + See `jj config edit` if you'd like to immediately edit a file. **Usage:** `jj config path <--user|--repo|--workspace>` diff --git a/cli/tests/common/test_environment.rs b/cli/tests/common/test_environment.rs index db0c680c0..fc98d4779 100644 --- a/cli/tests/common/test_environment.rs +++ b/cli/tests/common/test_environment.rs @@ -162,6 +162,16 @@ impl TestEnvironment { cmd.env(key, value); } + if cfg!(windows) { + // Windows uses `TEMP` to create temporary directories, which we need for some + // tests. + if let Ok(tmp_var) = std::env::var("TEMP") { + cmd.env("TEMP", tmp_var); + } + // Ensure that our tests don't write to the real %APPDATA%. + cmd.env("APPDATA", self.home_dir.join(".config")); + } + cmd } diff --git a/cli/tests/test_alias.rs b/cli/tests/test_alias.rs index 079e0baa2..6520a2f0e 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 64dfc1116..172eabcd8 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -339,7 +339,7 @@ fn test_config_list_origin() { ]); 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 $TEST_ENV/home/.config/jj/repos/0757f5ec8418b4f0983d/config.toml user.name = "Test User" # env user.email = "test.user@example.com" # env debug.commit-timestamp = "2001-02-03T04:05:11+07:00" # env @@ -415,10 +415,9 @@ 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, "repo"]) + .success(); let output = work_dir.run_jj(["config", "list", config_key]); insta::assert_snapshot!(output, @r#" merge-tools.vimdiff.program = "repo" @@ -495,10 +494,9 @@ 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, "repo"]) + .success(); let output = work_dir.run_jj(["config", "list", config_key]); insta::assert_snapshot!(output, @r#" ui.editor = "repo" @@ -561,13 +559,9 @@ 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, "main-repo"]) + .success(); let output = main_dir.run_jj(["config", "list", config_key]); insta::assert_snapshot!(output, @r#" ui.editor = "main-repo" @@ -724,7 +718,8 @@ fn test_config_set_for_repo() { .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"); + let config_dir = test_env.work_dir("home/.config/jj/repos/8e4fac809cbb3b162c95"); + let repo_config_toml = config_dir.read_file("config.toml"); insta::assert_snapshot!(repo_config_toml, @r#" #:schema https://docs.jj-vcs.dev/latest/config-schema.json @@ -733,6 +728,23 @@ fn test_config_set_for_repo() { [test-table] foo = true "#); + + std::fs::remove_dir_all(config_dir.root()).unwrap(); + let output = work_dir.run_jj(["config", "path", "--repo"]); + insta::assert_snapshot!(output, @r" + $TEST_ENV/home/.config/jj/repos/8e4fac809cbb3b162c95/config.toml + [EOF] + ------- stderr ------- + Warning: Per-repo config not found. Generating an empty one. + Per-repo config is stored in the same directory as your user config for security reasons. + If you work across multiple computers, you may want to keep your user config directory in sync. + [EOF] + "); + + // Check that it's regenerated the metadata. + assert!(config_dir.root().join("metadata.binpb").is_file()); + // But not the config file itself. + assert!(!config_dir.root().join("config.toml").is_file()); } #[test] @@ -752,7 +764,9 @@ fn test_config_set_for_workspace() { .success(); // Read workspace config - let workspace_config = work_dir.read_file(".jj/workspace-config.toml"); + let workspace_config = &test_env + .work_dir("home/.config/jj/workspaces/0757f5ec8418b4f0983d") + .read_file("config.toml"); insta::assert_snapshot!(workspace_config, @r#" #:schema https://docs.jj-vcs.dev/latest/config-schema.json @@ -968,7 +982,9 @@ fn test_config_unset_for_repo() { .run_jj(["config", "unset", "--repo", "test-key"]) .success(); - let repo_config_toml = work_dir.read_file(".jj/repo/config.toml"); + let repo_config_toml = &test_env + .work_dir("home/.config/jj/repos/8e4fac809cbb3b162c95") + .read_file("config.toml"); insta::assert_snapshot!(repo_config_toml, @""); } @@ -991,7 +1007,9 @@ fn test_config_unset_for_workspace() { .run_jj(["config", "unset", "--workspace", "foo"]) .success(); - let workspace_config = work_dir.read_file(".jj/workspace-config.toml"); + let workspace_config = &test_env + .work_dir("home/.config/jj/workspaces/0757f5ec8418b4f0983d") + .read_file("config.toml"); insta::assert_snapshot!(workspace_config, @""); } @@ -1055,17 +1073,21 @@ fn test_config_edit_repo() { let mut test_env = TestEnvironment::default(); 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()); + let repo_config_dir = test_env.home_dir().join(".config/jj/repos"); + assert!(!repo_config_dir.is_dir()); std::fs::write(edit_script, "dump-path path").unwrap(); + let work_dir = test_env.work_dir("repo"); work_dir.run_jj(["config", "edit", "--repo"]).success(); + let repo_config_path = test_env + .work_dir("home/.config/jj/repos/8e4fac809cbb3b162c95") + .root() + .join("config.toml"); + let edited_path = PathBuf::from(std::fs::read_to_string(test_env.env_root().join("path")).unwrap()); + assert!(repo_config_dir.is_dir()); assert_eq!(edited_path, dunce::simplified(&repo_config_path)); assert!(repo_config_path.exists(), "new file should be created"); } @@ -1090,7 +1112,7 @@ fn test_config_edit_invalid_config() { }); insta::assert_snapshot!(output, @r" ------- stderr ------- - Editing file: $TEST_ENV/repo/.jj/repo/config.toml + Editing file: $TEST_ENV/home/.config/jj/repos/8e4fac809cbb3b162c95/config.toml Warning: An error has been found inside the config: Caused by: 1: Configuration cannot be parsed as TOML document @@ -1120,7 +1142,7 @@ fn test_config_edit_invalid_config() { }); insta::assert_snapshot!(output, @r" ------- stderr ------- - Editing file: $TEST_ENV/repo/.jj/repo/config.toml + Editing file: $TEST_ENV/home/.config/jj/repos/8e4fac809cbb3b162c95/config.toml Warning: An error has been found inside the config: Caused by: 1: Configuration cannot be parsed as TOML document @@ -1167,7 +1189,7 @@ fn test_config_path() { ); insta::assert_snapshot!(work_dir.run_jj(["config", "path", "--repo"]), @r" - $TEST_ENV/repo/.jj/repo/config.toml + $TEST_ENV/home/.config/jj/repos/ffdaa62087a280bddc5e/config.toml [EOF] "); assert!( @@ -1183,7 +1205,7 @@ fn test_config_path() { "); insta::assert_snapshot!(work_dir.run_jj(["config", "path", "--workspace"]), @r" - $TEST_ENV/repo/.jj/workspace-config.toml + $TEST_ENV/home/.config/jj/workspaces/d043564ef93650b06a70/config.toml [EOF] "); assert!( diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs index ba8341a0b..cf9f767ef 100644 --- a/cli/tests/test_global_opts.rs +++ b/cli/tests/test_global_opts.rs @@ -492,7 +492,10 @@ 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", "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 ae060efae..fde111f9e 100644 --- a/cli/tests/test_log_command.rs +++ b/cli/tests/test_log_command.rs @@ -1193,9 +1193,9 @@ fn test_default_revset_per_repo() { work_dir.write_file("file1", "foo\n"); 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", "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 90c821eb2..de2698ee1 100644 --- a/docs/config.md +++ b/docs/config.md @@ -13,11 +13,13 @@ 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`, or found + with `jj config path --repo`. For security reasons, they are not located inside + the 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`, + or found with `jj config path --workspace`. For security reasons, they are not + located inside the workspace. - Settings [specified in the command-line](#specifying-config-on-the-command-line). diff --git a/docs/design/secure-config.md b/docs/design/secure-config.md index 7c37e2e1c..cf14a7816 100644 --- a/docs/design/secure-config.md +++ b/docs/design/secure-config.md @@ -2,6 +2,8 @@ Author: [Matt Stark](mailto:msta@google.com) +Status: Implemented + ## The problem An attacker that has control over your jj configuration has full control over diff --git a/docs/gerrit.md b/docs/gerrit.md index d9d16d091..5b46e9d02 100644 --- a/docs/gerrit.md +++ b/docs/gerrit.md @@ -29,8 +29,9 @@ $ jj git remote add gerrit https://review.gerrithub.io/yourname/yourproject $ 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: +If you used option 2 you can configure default values in your repository config +by appending the following lines to your config file, like so (to do this for +a specific repo, run `jj config edit --repo`): ```toml [gerrit] diff --git a/docs/github.md b/docs/github.md index 7584d339e..f1d76f94f 100644 --- a/docs/github.md +++ b/docs/github.md @@ -266,8 +266,8 @@ 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 --user|repo|workspace`): ```toml [git] diff --git a/lib/gen-protos/src/main.rs b/lib/gen-protos/src/main.rs index 22c2740b6..4cb38a8dc 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", + "secure_config.proto", ]; let root = Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap(); diff --git a/lib/src/lib.rs b/lib/src/lib.rs index aed05c51c..c3e390edf 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -89,6 +89,7 @@ mod revset_parser; pub mod rewrite; #[cfg(feature = "testing")] pub mod secret_backend; +pub mod secure_config; pub mod settings; pub mod signing; pub mod tree_merge; diff --git a/lib/src/protos/mod.rs b/lib/src/protos/mod.rs index 9bb5313d4..f7e6938f2 100644 --- a/lib/src/protos/mod.rs +++ b/lib/src/protos/mod.rs @@ -8,6 +8,9 @@ pub mod git_store { pub mod local_working_copy { include!("local_working_copy.rs"); } +pub mod secure_config { + include!("secure_config.rs"); +} pub mod simple_op_store { include!("simple_op_store.rs"); } diff --git a/lib/src/protos/secure_config.proto b/lib/src/protos/secure_config.proto new file mode 100644 index 000000000..859463a1e --- /dev/null +++ b/lib/src/protos/secure_config.proto @@ -0,0 +1,25 @@ +// 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 secure_config; + +// Metadata about the configuration files for jj +message ConfigMetadata { + // The absolute path to the workspace's .jj or .jj/repo directory for + // workspaces and repos respectively. + // Access via file_util::path_to/from_bytes + optional bytes path = 1; +} \ No newline at end of file diff --git a/lib/src/protos/secure_config.rs b/lib/src/protos/secure_config.rs new file mode 100644 index 000000000..7f01fcf43 --- /dev/null +++ b/lib/src/protos/secure_config.rs @@ -0,0 +1,10 @@ +// This file is @generated by prost-build. +/// Metadata about the configuration files for jj +#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] +pub struct ConfigMetadata { + /// The absolute path to the workspace's .jj or .jj/repo directory for + /// workspaces and repos respectively. + /// Access via file_util::path_to/from_bytes + #[prost(bytes = "vec", optional, tag = "1")] + pub path: ::core::option::Option<::prost::alloc::vec::Vec>, +} diff --git a/lib/src/secure_config.rs b/lib/src/secure_config.rs new file mode 100644 index 000000000..40f2fe06b --- /dev/null +++ b/lib/src/secure_config.rs @@ -0,0 +1,590 @@ +// 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 mechanism to access config files for a repo securely. + +use std::cell::RefCell; +use std::fs; +use std::io::ErrorKind::NotFound; +use std::io::Write as _; +use std::path::Path; +use std::path::PathBuf; + +use prost::Message as _; +use rand::Rng as _; +use rand_chacha::ChaCha20Rng; +use tempfile::NamedTempFile; +use thiserror::Error; + +use crate::file_util::BadPathEncoding; +use crate::file_util::IoResultExt as _; +use crate::file_util::PathError; +use crate::file_util::path_from_bytes; +use crate::file_util::path_to_bytes; +use crate::hex_util::encode_hex; +use crate::protos::secure_config::ConfigMetadata; + +const CONFIG_FILE: &str = "config.toml"; +const METADATA_FILE: &str = "metadata.binpb"; +const CONFIG_ID_BYTES: usize = 10; +#[cfg(not(unix))] +const CONTENT_PREFIX: &str = r###"# DO NOT EDIT. +# This file is for old versions of jj. +# It will be used for jj >= v0.37. +# Use `jj config path` or `jj config edit` to find and edit the new file + +"###; +const CONFIG_NOT_FOUND: &str = r###"Per-repo config not found. Generating an empty one. +Per-repo config is stored in the same directory as your user config for security reasons. +If you work across multiple computers, you may want to keep your user config directory in sync."###; + +/// A mechanism to access config files for a repo securely. +#[derive(Clone, Debug)] +pub struct SecureConfig { + /// Technically this is either a repo or a workspace. + repo_dir: PathBuf, + /// The name of the config id file. + config_id_name: &'static str, + /// The name of the legacy config file. + legacy_config_name: &'static str, + /// A cache of the output \[maybe_\]load_config + cache: RefCell, ConfigMetadata)>>, +} + +/// An error when attempting to load config from disk. +#[derive(Error, Debug)] +pub enum SecureConfigError { + /// Failed to read / write to the specified path + #[error(transparent)] + PathError(#[from] PathError), + + /// Failed to decode the user configuration file. + #[error(transparent)] + DecodeError(#[from] prost::DecodeError), + + /// The path failed to decode to bytes. + #[error(transparent)] + BadPathEncoding(#[from] BadPathEncoding), + + /// The config ID isn't CONFIG_ID_BYTES * 2 hex chars. + #[error("Found an invalid config ID")] + BadConfigIdError, +} + +/// The path to the config file for a secure config. +/// Also contains metadata about the repo and info to be displayed to the +/// user. +#[derive(Clone, Debug, Default)] +pub struct LoadedSecureConfig { + /// The path to the config file. + /// Can be None if the config-ID is not generated. + pub config_file: Option, + /// The metadata for the config. + pub metadata: ConfigMetadata, + /// Any warnings that we want to be reported to the user. + pub warnings: Vec, +} + +fn atomic_write(path: &Path, content: &[u8]) -> Result<(), SecureConfigError> { + let mut temp_file = NamedTempFile::new_in(path.parent().unwrap()).context(path)?; + temp_file.write_all(content).context(path)?; + temp_file.persist(path).map_err(|e| PathError { + path: path.to_path_buf(), + source: e.error, + })?; + Ok(()) +} + +fn generate_config_id(rng: &mut ChaCha20Rng) -> String { + encode_hex(&rng.random::<[u8; CONFIG_ID_BYTES]>()) +} + +fn update_metadata(config_dir: &Path, metadata: &ConfigMetadata) -> Result<(), SecureConfigError> { + let metadata_path = config_dir.join(METADATA_FILE); + atomic_write(&metadata_path, &metadata.encode_to_vec())?; + Ok(()) +} + +impl SecureConfig { + /// Creates a secure config. + fn new( + repo_dir: PathBuf, + config_id_name: &'static str, + legacy_config_name: &'static str, + ) -> Self { + Self { + repo_dir, + config_id_name, + legacy_config_name, + cache: RefCell::new(None), + } + } + + /// Creates a secure config for a repo. Takes the .jj/repo directory. + pub fn new_repo(repo_dir: PathBuf) -> Self { + Self::new(repo_dir, "config-id", "config.toml") + } + + /// Creates a secure config for a workspace. Takes the .jj directory. + pub fn new_workspace(workspace_dir: PathBuf) -> Self { + Self::new( + workspace_dir, + "workspace-config-id", + "workspace-config.toml", + ) + } + + fn generate_config( + &self, + root_config_dir: &Path, + config_id: &str, + content: Option<&[u8]>, + metadata: &ConfigMetadata, + ) -> Result { + let config_dir = root_config_dir.join(config_id); + let config_path = config_dir.join(CONFIG_FILE); + fs::create_dir_all(&config_dir).context(&config_dir)?; + update_metadata(&config_dir, metadata)?; + if let Some(content) = content { + fs::write(&config_path, content).context(&config_path)?; + } + + // Write the config ID atomically. A half-formed config ID would be very bad. + atomic_write( + &self.repo_dir.join(self.config_id_name), + config_id.as_bytes(), + )?; + Ok(config_path) + } + + fn generate_initial_config( + &self, + root_config_dir: &Path, + config_id: &str, + ) -> Result<(PathBuf, ConfigMetadata), SecureConfigError> { + let metadata = ConfigMetadata { + path: path_to_bytes(&self.repo_dir).ok().map(|b| b.to_vec()), + }; + let path = self.generate_config(root_config_dir, config_id, None, &metadata)?; + Ok((path, metadata)) + } + + /// Validates that the metadata path matches the repo path. + /// If there's a mismatch, takes appropriate action. + /// Returns the updated config dir and metadata. + fn handle_metadata_path( + &self, + rng: &mut ChaCha20Rng, + root_config_dir: &Path, + config_dir: PathBuf, + mut metadata: ConfigMetadata, + ) -> Result { + let encoded = path_to_bytes(&self.repo_dir).ok(); + let got = metadata + .path + .as_deref() + .map(|p| path_from_bytes(p)) + .transpose()?; + + if got == encoded.is_some().then_some(self.repo_dir.as_path()) { + return Ok(LoadedSecureConfig { + config_file: Some(config_dir.join(CONFIG_FILE)), + metadata, + warnings: vec![], + }); + } + let got = match got { + Some(d) if d.is_dir() => d.to_path_buf(), + _ => { + // The old repo does not exist. Assume the user moved it. + metadata.path = encoded.map(|b| b.to_vec()); + update_metadata(&config_dir, &metadata)?; + return Ok(LoadedSecureConfig { + config_file: Some(config_dir.join(CONFIG_FILE)), + metadata, + warnings: vec![], + }); + } + }; + // We attempt to create a temporary file in the new repo. + // If it fails, we have readonly access to a repo, so we do nothing. + if let Ok(tmp) = NamedTempFile::new_in(&self.repo_dir) { + // If we write to the new repo and it shows up in the old one, + // we can skip this step, since it's not a copy. + if !got.join(tmp.path().file_name().unwrap()).exists() { + // We now assume the repo was copied. Since the repo was copied, + // the config should be copied too, rather than sharing the + // config with what it copied from. + let old_config_path = config_dir.join(CONFIG_FILE); + metadata.path = encoded.map(|b| b.to_vec()); + let old_config_content = fs::read(&old_config_path).context(&old_config_path)?; + let config_path = self.generate_config( + root_config_dir, + &generate_config_id(rng), + Some(&old_config_content), + &metadata, + )?; + return Ok(LoadedSecureConfig { + config_file: Some(config_path.clone()), + metadata, + warnings: vec![format!( + "Your repo appears to have been copied from {} to {}. The corresponding \ + repo config file has also been copied.", + got.display(), + &self.repo_dir.display() + )], + }); + }; + } + Ok(LoadedSecureConfig { + config_file: Some(config_dir.join(CONFIG_FILE)), + metadata, + warnings: vec![], + }) + } + + #[cfg(unix)] + fn update_legacy_config_file( + &self, + new_config: &Path, + _content: &[u8], + ) -> Result<(), SecureConfigError> { + let legacy_config = self.repo_dir.join(self.legacy_config_name); + // Make old versions and new versions of jj share the same config file. + fs::remove_file(&legacy_config).context(&legacy_config)?; + std::os::unix::fs::symlink(new_config, &legacy_config).context(&legacy_config)?; + Ok(()) + } + + #[cfg(not(unix))] + fn update_legacy_config_file( + &self, + _new_config: &Path, + content: &[u8], + ) -> Result<(), SecureConfigError> { + let legacy_config = self.repo_dir.join(self.legacy_config_name); + // I considered making this readonly, but that would prevent you from + // updating the config with old versions of jj. + // In the future, we consider something a little more robust, where as + // the non-legacy config changes, we propagate that to the legacy config. + // However, it seems a little overkill, considering it only affects windows + // users who use multiple versions of jj at once, and only for a year. + let mut new_content = CONTENT_PREFIX.as_bytes().to_vec(); + new_content.extend_from_slice(content); + fs::write(&legacy_config, new_content).context(&legacy_config)?; + Ok(()) + } + + /// Migrates the legacy config, if it exists. + fn maybe_migrate_legacy_config( + &self, + rng: &mut ChaCha20Rng, + root_config_dir: &Path, + ) -> Result { + // TODO: This function should be updated in jj 0.49 to no longer + // automatically migrate repos, but instead print out a warning. + let legacy_config = self.repo_dir.join(self.legacy_config_name); + let config = match fs::read(&legacy_config).context(&legacy_config) { + Ok(config) => config, + // No legacy config files found. + Err(e) if e.source.kind() == NotFound => return Ok(Default::default()), + Err(e) => return Err(e.into()), + }; + let metadata = ConfigMetadata { + path: path_to_bytes(&self.repo_dir).ok().map(|b| b.to_vec()), + }; + let config_file = self.generate_config( + root_config_dir, + &generate_config_id(rng), + Some(&config), + &metadata, + )?; + self.update_legacy_config_file(&config_file, &config)?; + Ok(LoadedSecureConfig { + warnings: vec![format!( + "Your config file has been migrated from {} to {}. You can edit the new file with \ + `jj config edit`", + legacy_config.display(), + config_file.display(), + )], + config_file: Some(config_file), + metadata, + }) + } + + /// Determines the path to the config, and any metadata associated with it. + /// If no config exists, the path will be None. + pub fn maybe_load_config( + &self, + rng: &mut ChaCha20Rng, + root_config_dir: &Path, + ) -> Result { + if let Some(cache) = self.cache.borrow().as_ref() { + return Ok(LoadedSecureConfig { + config_file: cache.0.clone(), + metadata: cache.1.clone(), + warnings: vec![], + }); + } + let config_id_path = self.repo_dir.join(self.config_id_name); + let loaded = match fs::read_to_string(&config_id_path).context(&config_id_path) { + Ok(config_id) => { + if config_id.len() != CONFIG_ID_BYTES * 2 + || !config_id.chars().all(|c| c.is_ascii_hexdigit()) + { + return Err(SecureConfigError::BadConfigIdError); + } + let config_dir = root_config_dir.join(&config_id); + let metadata_path = config_dir.join(METADATA_FILE); + match fs::read(&metadata_path).context(&metadata_path) { + Ok(buf) => self.handle_metadata_path( + rng, + root_config_dir, + config_dir, + ConfigMetadata::decode(buf.as_slice())?, + )?, + Err(e) if e.source.kind() == NotFound => { + let (path, metadata) = + self.generate_initial_config(root_config_dir, &config_id)?; + LoadedSecureConfig { + config_file: Some(path), + metadata, + warnings: vec![CONFIG_NOT_FOUND.to_string()], + } + } + Err(e) => return Err(e.into()), + } + } + Err(e) if e.source.kind() == NotFound => { + self.maybe_migrate_legacy_config(rng, root_config_dir)? + } + Err(e) => return Err(SecureConfigError::PathError(e)), + }; + *self.cache.borrow_mut() = Some((loaded.config_file.clone(), loaded.metadata.clone())); + Ok(loaded) + } + + /// Determines the path to the config, and any metadata associated with it. + /// If no config exists, an empty config file will be generated. + pub fn load_config( + &self, + rng: &mut ChaCha20Rng, + root_config_dir: &Path, + ) -> Result { + let mut loaded = self.maybe_load_config(rng, root_config_dir)?; + if loaded.config_file.is_none() { + let (path, metadata) = + self.generate_initial_config(root_config_dir, &generate_config_id(rng))?; + *self.cache.borrow_mut() = Some((Some(path.clone()), metadata.clone())); + loaded.config_file = Some(path); + loaded.metadata = metadata; + } + Ok(loaded) + } +} + +#[cfg(test)] +mod tests { + use std::ffi::OsStr; + + use rand::SeedableRng as _; + use tempfile::TempDir; + + use super::*; + + struct TestEnv { + _td: TempDir, + rng: ChaCha20Rng, + config: SecureConfig, + repo_dir: PathBuf, + config_dir: PathBuf, + } + + impl TestEnv { + fn new() -> Self { + let td = TempDir::new().unwrap(); + let repo_dir = td.path().join("repo"); + fs::create_dir(&repo_dir).unwrap(); + let config_dir = td.path().join("config"); + fs::create_dir(&config_dir).unwrap(); + Self { + _td: td, + rng: ChaCha20Rng::seed_from_u64(0), + config: SecureConfig::new(repo_dir.clone(), "config-id", "legacy-config.toml"), + repo_dir, + config_dir, + } + } + + fn secure_config_for_dir(&self, d: PathBuf) -> SecureConfig { + SecureConfig::new(d, "config-id", "legacy-config.toml") + } + } + + #[test] + fn test_no_initial_config() { + let mut env = TestEnv::new(); + + // We shouldn't generate the config. + let loaded = env + .config + .maybe_load_config(&mut env.rng, &env.config_dir) + .unwrap(); + assert_eq!(loaded.config_file, None); + assert_eq!(loaded.metadata, Default::default()); + assert!(loaded.warnings.is_empty()); + // The cache entry should be filled. + assert!(env.config.cache.borrow().is_some()); + + // load_config should generate the config if it previously didn't exist. + let loaded = env + .config + .load_config(&mut env.rng, &env.config_dir) + .unwrap(); + let path = loaded.config_file.unwrap(); + let components: Vec<_> = path.components().rev().collect(); + assert_eq!( + components[0], + std::path::Component::Normal(OsStr::new("config.toml")) + ); + assert_eq!( + components[2], + std::path::Component::Normal(OsStr::new("config")) + ); + assert!(!loaded.metadata.path.as_deref().unwrap().is_empty()); + assert!(loaded.warnings.is_empty()); + + // load_config should leave it untouched if it did exist. + // Empty the cache to ensure the function is actually being tested + assert!(env.config.cache.borrow().is_some()); + *env.config.cache.borrow_mut() = None; + let loaded2 = env + .config + .load_config(&mut env.rng, &env.config_dir) + .unwrap(); + assert_eq!(loaded2.config_file.unwrap(), path); + assert_eq!(loaded2.metadata, loaded.metadata); + assert!(loaded2.warnings.is_empty()); + } + + #[test] + fn test_migrate_legacy_config() { + let mut env = TestEnv::new(); + + let legacy_config = env.repo_dir.join("legacy-config.toml"); + fs::write(&legacy_config, "config").unwrap(); + let loaded = env + .config + .maybe_load_config(&mut env.rng, &env.config_dir) + .unwrap(); + assert!(loaded.config_file.is_some()); + assert!(!loaded.metadata.path.unwrap().is_empty()); + assert_eq!( + fs::read_to_string(loaded.config_file.as_deref().unwrap()).unwrap(), + "config" + ); + assert!(!loaded.warnings.is_empty()); + + // On unix, it should be a symlink. + if cfg!(unix) { + fs::write(loaded.config_file.as_deref().unwrap(), "new").unwrap(); + assert_eq!(fs::read_to_string(&legacy_config).unwrap(), "new"); + } + } + + #[test] + fn test_repo_moved() { + let mut env = TestEnv::new(); + let loaded = env + .config + .load_config(&mut env.rng, &env.config_dir) + .unwrap(); + let path = loaded.config_file.unwrap(); + + let dest = env.repo_dir.parent().unwrap().join("moved"); + fs::rename(&env.repo_dir, &dest).unwrap(); + let config = env.secure_config_for_dir(dest); + let loaded2 = config.load_config(&mut env.rng, &env.config_dir).unwrap(); + assert_eq!(loaded2.config_file.unwrap(), path); + assert_ne!(loaded.metadata.path, loaded2.metadata.path); + assert!(loaded2.warnings.is_empty()); + } + + #[test] + fn test_repo_copied() { + let mut env = TestEnv::new(); + let loaded = env + .config + .load_config(&mut env.rng, &env.config_dir) + .unwrap(); + let path = loaded.config_file.unwrap(); + fs::write(&path, "config").unwrap(); + + let dest = env.repo_dir.parent().unwrap().join("copied"); + fs::create_dir(&dest).unwrap(); + fs::copy(env.repo_dir.join("config-id"), dest.join("config-id")).unwrap(); + let config = env.secure_config_for_dir(dest); + let loaded2 = config.load_config(&mut env.rng, &env.config_dir).unwrap(); + let path2 = loaded2.config_file.unwrap(); + assert_ne!(path, path2); + assert_eq!(fs::read_to_string(path2).unwrap(), "config"); + assert_ne!(loaded.metadata.path, loaded2.metadata.path); + // We should get a warning about the repo having been copied. + assert!(!loaded2.warnings.is_empty()); + } + + // This feature works on windows as well, it just isn't easy to replicate with a + // test. + #[cfg(unix)] + #[test] + fn test_repo_aliased() { + let mut env = TestEnv::new(); + let loaded = env + .config + .load_config(&mut env.rng, &env.config_dir) + .unwrap(); + let path = loaded.config_file.unwrap(); + + let dest = env.repo_dir.parent().unwrap().join("copied"); + std::os::unix::fs::symlink(&env.repo_dir, &dest).unwrap(); + let config = env.secure_config_for_dir(dest); + let loaded2 = config.load_config(&mut env.rng, &env.config_dir).unwrap(); + assert_eq!(loaded2.config_file.unwrap(), path); + assert_eq!(loaded.metadata.path, loaded2.metadata.path); + assert!(loaded2.warnings.is_empty()); + } + + #[test] + fn test_missing_config() { + let mut env = TestEnv::new(); + let loaded = env + .config + .load_config(&mut env.rng, &env.config_dir) + .unwrap(); + let path = loaded.config_file.unwrap(); + + fs::remove_dir_all(path.parent().unwrap()).unwrap(); + *env.config.cache.borrow_mut() = None; + + let loaded2 = env + .config + .load_config(&mut env.rng, &env.config_dir) + .unwrap(); + assert_eq!(loaded2.config_file.unwrap(), path); + assert_eq!(loaded.metadata.path, loaded2.metadata.path); + // It should have recreated the directory. + assert!(path.parent().unwrap().is_dir()); + assert!(!loaded2.warnings.is_empty()); + } +}