Skip to content

Commit 721cf57

Browse files
committed
config: Implement secure-config design doc.
See docs/design/secure-config.md for many more details. Fixes #3303 Fixes #1595
1 parent a3074ef commit 721cf57

File tree

22 files changed

+646
-74
lines changed

22 files changed

+646
-74
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
1919
filesystem's behavior, but this can be overridden manually by setting
2020
`working-copy.exec-bit-change = "respect" | "ignore"`.
2121

22+
* Per-repo and per-workspace config is now stored outside the repo, for security
23+
reasons. This is not a breaking change because we automatically migrate
24+
legacy repos to this new format. `.jj/repo/config.toml` and
25+
`.jj/workspace-config.toml` should no longer be used.
26+
2227
### Fixed bugs
2328

2429
## [0.36.0] - 2025-12-03

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ gix = { version = "0.75.0", default-features = false, features = [
5858
] }
5959
globset = "0.4.18"
6060
hashbrown = { version = "0.16.1", default-features = false, features = ["inline-more"] }
61+
hex = "0.4.3"
6162
ignore = "0.4.25"
6263
indexmap = { version = "2.12.1", features = ["serde"] }
6364
indoc = "2.0.7"

cli/src/cli_util.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ impl CommandHelper {
354354
///
355355
/// This may be different from the settings for new workspace created by
356356
/// e.g. `jj git init`. There may be conditional variables and repo config
357-
/// `.jj/repo/config.toml` loaded for the cwd workspace.
357+
/// loaded for the cwd workspace.
358358
pub fn settings(&self) -> &UserSettings {
359359
&self.data.settings
360360
}

cli/src/command_error.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,9 @@ impl From<ConfigLoadError> for CommandError {
277277
ConfigLoadError::Parse { source_path, .. } => source_path
278278
.as_ref()
279279
.map(|path| format!("Check the config file: {}", path.display())),
280+
ConfigLoadError::SecureConfigError(e) => {
281+
Some(format!("Failed to load secure config: {e}"))
282+
}
280283
};
281284
let mut cmd_err = config_error(err);
282285
cmd_err.extend_hints(hint);

cli/src/commands/config/mod.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ mod path;
1919
mod set;
2020
mod unset;
2121

22-
use std::path::Path;
22+
use std::path::PathBuf;
2323

2424
use itertools::Itertools as _;
2525
use jj_lib::config::ConfigFile;
@@ -73,21 +73,24 @@ impl ConfigLevelArgs {
7373
}
7474
}
7575

76-
fn config_paths<'a>(&self, config_env: &'a ConfigEnv) -> Result<Vec<&'a Path>, CommandError> {
76+
fn config_paths(&self, config_env: &ConfigEnv) -> Result<Vec<PathBuf>, CommandError> {
7777
if self.user {
78-
let paths = config_env.user_config_paths().collect_vec();
78+
let paths = config_env
79+
.user_config_paths()
80+
.map(|p| p.to_path_buf())
81+
.collect_vec();
7982
if paths.is_empty() {
8083
return Err(user_error("No user config path found"));
8184
}
8285
Ok(paths)
8386
} else if self.repo {
8487
config_env
85-
.repo_config_path()
88+
.repo_config_path()?
8689
.map(|p| vec![p])
8790
.ok_or_else(|| user_error("No repo config path found"))
8891
} else if self.workspace {
8992
config_env
90-
.workspace_config_path()
93+
.workspace_config_path()?
9194
.map(|p| vec![p])
9295
.ok_or_else(|| user_error("No workspace config path found"))
9396
} else {

cli/src/commands/config/path.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub fn cmd_config_path(
4141
args: &ConfigPathArgs,
4242
) -> Result<(), CommandError> {
4343
for config_path in args.level.config_paths(command.config_env())? {
44-
let path_bytes = file_util::path_to_bytes(config_path).map_err(user_error)?;
44+
let path_bytes = file_util::path_to_bytes(&config_path).map_err(user_error)?;
4545
ui.stdout().write_all(path_bytes)?;
4646
writeln!(ui.stdout())?;
4747
}

cli/src/config.rs

Lines changed: 66 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ use jj_lib::config::ConfigSource;
3636
use jj_lib::config::ConfigValue;
3737
use jj_lib::config::StackedConfig;
3838
use jj_lib::dsl_util;
39+
use jj_lib::secure_config::SecureConfig;
3940
use regex::Captures;
4041
use regex::Regex;
4142
use serde::Serialize as _;
@@ -271,6 +272,10 @@ struct UnresolvedConfigEnv {
271272
}
272273

273274
impl UnresolvedConfigEnv {
275+
fn root_config_dir(&self) -> Option<PathBuf> {
276+
self.config_dir.as_deref().map(|c| c.join("jj"))
277+
}
278+
274279
fn resolve(self) -> Vec<ConfigPath> {
275280
if let Some(paths) = self.jj_config {
276281
return split_paths(&paths)
@@ -320,11 +325,12 @@ impl UnresolvedConfigEnv {
320325
#[derive(Clone, Debug)]
321326
pub struct ConfigEnv {
322327
home_dir: Option<PathBuf>,
328+
root_config_dir: Option<PathBuf>,
323329
repo_path: Option<PathBuf>,
324330
workspace_path: Option<PathBuf>,
325331
user_config_paths: Vec<ConfigPath>,
326-
repo_config_path: Option<ConfigPath>,
327-
workspace_config_path: Option<ConfigPath>,
332+
repo_config: Option<SecureConfig>,
333+
workspace_config: Option<SecureConfig>,
328334
command: Option<String>,
329335
hostname: Option<String>,
330336
}
@@ -349,11 +355,12 @@ impl ConfigEnv {
349355
};
350356
Self {
351357
home_dir,
358+
root_config_dir: env.root_config_dir(),
352359
repo_path: None,
353360
workspace_path: None,
354361
user_config_paths: env.resolve(),
355-
repo_config_path: None,
356-
workspace_config_path: None,
362+
repo_config: None,
363+
workspace_config: None,
357364
command: None,
358365
hostname: whoami::fallible::hostname().ok(),
359366
}
@@ -424,21 +431,33 @@ impl ConfigEnv {
424431
/// Sets the directory where repo-specific config file is stored. The path
425432
/// is usually `.jj/repo`.
426433
pub fn reset_repo_path(&mut self, path: &Path) {
434+
if self.repo_path.as_deref() != Some(path)
435+
&& let Some(root_config_dir) = self.root_config_dir.as_deref()
436+
{
437+
self.repo_config = Some(SecureConfig::new(
438+
path.to_owned(),
439+
root_config_dir.join("repos"),
440+
"repo-config-id",
441+
"config.toml",
442+
));
443+
}
427444
self.repo_path = Some(path.to_owned());
428-
self.repo_config_path = Some(ConfigPath::new(path.join("config.toml")));
429445
}
430446

431-
/// Returns a path to the repo-specific config file.
432-
pub fn repo_config_path(&self) -> Option<&Path> {
433-
self.repo_config_path.as_ref().map(|p| p.as_path())
447+
/// Returns a path to the existing repo-specific config file.
448+
fn maybe_repo_config_path(&self) -> Result<Option<PathBuf>, ConfigLoadError> {
449+
Ok(match &self.repo_config {
450+
Some(config) => config.maybe_load_config()?.0,
451+
None => None,
452+
})
434453
}
435454

436455
/// Returns a path to the existing repo-specific config file.
437-
fn existing_repo_config_path(&self) -> Option<&Path> {
438-
match self.repo_config_path {
439-
Some(ref path) if path.exists() => Some(path.as_path()),
440-
_ => None,
441-
}
456+
pub fn repo_config_path(&self) -> Result<Option<PathBuf>, ConfigLoadError> {
457+
Ok(match &self.repo_config {
458+
Some(config) => Some(config.load_config()?.0),
459+
None => None,
460+
})
442461
}
443462

444463
/// Returns repo configuration files for modification. Instantiates one if
@@ -455,7 +474,7 @@ impl ConfigEnv {
455474
}
456475

457476
fn new_repo_config_file(&self) -> Result<Option<ConfigFile>, ConfigLoadError> {
458-
self.repo_config_path()
477+
self.repo_config_path()?
459478
// The path doesn't usually exist, but we shouldn't overwrite it
460479
// with an empty config if it did exist.
461480
.map(|path| ConfigFile::load_or_empty(ConfigSource::Repo, path))
@@ -467,32 +486,43 @@ impl ConfigEnv {
467486
#[instrument]
468487
pub fn reload_repo_config(&self, config: &mut RawConfig) -> Result<(), ConfigLoadError> {
469488
config.as_mut().remove_layers(ConfigSource::Repo);
470-
if let Some(path) = self.existing_repo_config_path() {
489+
if let Some(path) = self.maybe_repo_config_path()? {
471490
config.as_mut().load_file(ConfigSource::Repo, path)?;
472491
}
473492
Ok(())
474493
}
475494

476-
/// Sets the directory for the workspace and the workspace-specific config
477-
/// file.
495+
/// Sets the directory where repo-specific config file is stored. The path
496+
/// is usually `.jj/repo`.
478497
pub fn reset_workspace_path(&mut self, path: &Path) {
498+
if self.workspace_path.as_deref() != Some(path)
499+
&& let Some(root_config_dir) = self.root_config_dir.as_deref()
500+
{
501+
self.workspace_config = Some(SecureConfig::new(
502+
path.to_owned(),
503+
root_config_dir.join("workspaces"),
504+
"workspace-config-id",
505+
"workspace-config.toml",
506+
));
507+
}
479508
self.workspace_path = Some(path.to_owned());
480-
self.workspace_config_path = Some(ConfigPath::new(
481-
path.join(".jj").join("workspace-config.toml"),
482-
));
483509
}
484510

485-
/// Returns a path to the workspace-specific config file.
486-
pub fn workspace_config_path(&self) -> Option<&Path> {
487-
self.workspace_config_path.as_ref().map(|p| p.as_path())
511+
/// Returns a path to the workspace-specific config file, if it exists.
512+
fn maybe_workspace_config_path(&self) -> Result<Option<PathBuf>, ConfigLoadError> {
513+
Ok(match &self.workspace_config {
514+
Some(config) => config.maybe_load_config()?.0,
515+
None => None,
516+
})
488517
}
489518

490-
/// Returns a path to the existing workspace-specific config file.
491-
fn existing_workspace_config_path(&self) -> Option<&Path> {
492-
match self.workspace_config_path {
493-
Some(ref path) if path.exists() => Some(path.as_path()),
494-
_ => None,
495-
}
519+
/// Returns a path to the existing workspace-specific config file, creating
520+
/// a new one if it doesn't exist.
521+
pub fn workspace_config_path(&self) -> Result<Option<PathBuf>, ConfigLoadError> {
522+
Ok(match &self.workspace_config {
523+
Some(config) => Some(config.load_config()?.0),
524+
None => None,
525+
})
496526
}
497527

498528
/// Returns workspace configuration files for modification. Instantiates one
@@ -511,7 +541,7 @@ impl ConfigEnv {
511541
}
512542

513543
fn new_workspace_config_file(&self) -> Result<Option<ConfigFile>, ConfigLoadError> {
514-
self.workspace_config_path()
544+
self.workspace_config_path()?
515545
.map(|path| ConfigFile::load_or_empty(ConfigSource::Workspace, path))
516546
.transpose()
517547
}
@@ -521,7 +551,7 @@ impl ConfigEnv {
521551
#[instrument]
522552
pub fn reload_workspace_config(&self, config: &mut RawConfig) -> Result<(), ConfigLoadError> {
523553
config.as_mut().remove_layers(ConfigSource::Workspace);
524-
if let Some(path) = self.existing_workspace_config_path() {
554+
if let Some(path) = self.maybe_workspace_config_path()? {
525555
config.as_mut().load_file(ConfigSource::Workspace, path)?;
526556
}
527557
Ok(())
@@ -565,8 +595,8 @@ fn config_files_for(
565595
/// 1. Default
566596
/// 2. Base environment variables
567597
/// 3. [User configs](https://docs.jj-vcs.dev/latest/config/)
568-
/// 4. Repo config `.jj/repo/config.toml`
569-
/// 5. Workspace config `.jj/workspace-config.toml`
598+
/// 4. Repo config
599+
/// 5. Workspace config
570600
/// 6. Override environment variables
571601
/// 7. Command-line arguments `--config` and `--config-file`
572602
///
@@ -1762,11 +1792,12 @@ mod tests {
17621792
};
17631793
ConfigEnv {
17641794
home_dir,
1795+
root_config_dir: None,
17651796
repo_path: None,
17661797
workspace_path: None,
17671798
user_config_paths: env.resolve(),
1768-
repo_config_path: None,
1769-
workspace_config_path: None,
1799+
repo_config: None,
1800+
workspace_config: None,
17701801
command: None,
17711802
hostname: None,
17721803
}

cli/tests/common/test_environment.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use std::ffi::OsStr;
1818
use std::ffi::OsString;
1919
use std::path::Path;
2020
use std::path::PathBuf;
21+
use std::sync::LazyLock;
2122

2223
use bstr::BString;
2324
use indoc::formatdoc;
@@ -32,6 +33,10 @@ use super::fake_diff_editor_path;
3233
use super::fake_editor_path;
3334
use super::to_toml_value;
3435

36+
static CONFIG_NAME: LazyLock<Regex> = LazyLock::new(|| {
37+
Regex::new(r#"(\$TEST_ENV/home/.config/jj/(:?repos|workspaces))/[0-9a-f]+"#).unwrap()
38+
});
39+
3540
pub struct TestEnvironment {
3641
_temp_dir: TempDir,
3742
env_root: PathBuf,
@@ -159,6 +164,8 @@ impl TestEnvironment {
159164
if let Ok(tmp_var) = std::env::var("TEMP") {
160165
cmd.env("TEMP", tmp_var);
161166
}
167+
// Ensure that our tests don't write to the real %APPDATA%.
168+
cmd.env("APPDATA", self.home_dir.join(".config"));
162169
}
163170

164171
cmd
@@ -284,6 +291,9 @@ impl TestEnvironment {
284291
.to_string();
285292
};
286293
}
294+
normalized = CONFIG_NAME
295+
.replace_all(&normalized, "$1/$$CONFIG_ID")
296+
.to_string();
287297
CommandOutputString { raw, normalized }
288298
}
289299

0 commit comments

Comments
 (0)