Skip to content

Commit dc31bd6

Browse files
committed
Use workspace trust in git subsystem
Replace default gitoxide's model of trust (directory ownership) with our own one. Closes #15594
1 parent 0805bc8 commit dc31bd6

8 files changed

Lines changed: 78 additions & 58 deletions

File tree

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.

helix-term/src/commands.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3541,16 +3541,18 @@ fn changed_file_picker(cx: &mut Context) {
35413541
.with_preview(|_editor, meta| Some((meta.path().into(), None)));
35423542
let injector = picker.injector();
35433543

3544-
cx.editor
3545-
.diff_providers
3546-
.clone()
3547-
.for_each_changed_file(cwd, move |change| match change {
3544+
let insecure = cx.editor.config().insecure;
3545+
cx.editor.diff_providers.clone().for_each_changed_file(
3546+
cwd,
3547+
insecure,
3548+
move |change| match change {
35483549
Ok(change) => injector.push(change).is_ok(),
35493550
Err(err) => {
35503551
status::report_blocking(err);
35513552
true
35523553
}
3553-
});
3554+
},
3555+
);
35543556
cx.push_layer(Box::new(overlaid(picker)));
35553557
}
35563558

helix-vcs/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ homepage.workspace = true
1212
[dependencies]
1313
helix-core = { path = "../helix-core" }
1414
helix-event = { path = "../helix-event" }
15+
helix-loader = { path = "../helix-loader" }
1516

1617
tokio = { version = "1", features = ["rt", "rt-multi-thread", "time", "sync", "parking_lot", "macros"] }
1718
parking_lot.workspace = true

helix-vcs/src/git.rs

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use anyhow::{bail, Context, Result};
22
use arc_swap::ArcSwap;
33
use gix::filter::plumbing::driver::apply::Delay;
4+
use helix_loader::find_workspace_in;
5+
use helix_loader::workspace_trust::{quick_query_workspace, TrustStatus};
46
use std::io::Read;
57
use std::path::Path;
68
use std::sync::Arc;
@@ -27,15 +29,15 @@ fn get_repo_dir(file: &Path) -> Result<&Path> {
2729
file.parent().context("file has no parent directory")
2830
}
2931

30-
pub fn get_diff_base(file: &Path) -> Result<Vec<u8>> {
32+
pub fn get_diff_base(file: &Path, insecure: bool) -> Result<Vec<u8>> {
3133
debug_assert!(!file.exists() || file.is_file());
3234
debug_assert!(file.is_absolute());
3335
let file = gix::path::realpath(file).context("resolve symlinks")?;
3436

3537
// TODO cache repository lookup
3638

3739
let repo_dir = get_repo_dir(&file)?;
38-
let repo = open_repo(repo_dir)
40+
let repo = open_repo(repo_dir, insecure)
3941
.context("failed to open git repo")?
4042
.to_thread_local();
4143
let head = repo.head_commit()?;
@@ -59,13 +61,13 @@ pub fn get_diff_base(file: &Path) -> Result<Vec<u8>> {
5961
}
6062
}
6163

62-
pub fn get_current_head_name(file: &Path) -> Result<Arc<ArcSwap<Box<str>>>> {
64+
pub fn get_current_head_name(file: &Path, insecure: bool) -> Result<Arc<ArcSwap<Box<str>>>> {
6365
debug_assert!(!file.exists() || file.is_file());
6466
debug_assert!(file.is_absolute());
6567
let file = gix::path::realpath(file).context("resolve symlinks")?;
6668

6769
let repo_dir = get_repo_dir(&file)?;
68-
let repo = open_repo(repo_dir)
70+
let repo = open_repo(repo_dir, insecure)
6971
.context("failed to open git repo")?
7072
.to_thread_local();
7173
let head_ref = repo.head_ref()?;
@@ -79,13 +81,17 @@ pub fn get_current_head_name(file: &Path) -> Result<Arc<ArcSwap<Box<str>>>> {
7981
Ok(Arc::new(ArcSwap::from_pointee(name.into_boxed_str())))
8082
}
8183

82-
pub fn for_each_changed_file(cwd: &Path, f: impl Fn(Result<FileChange>) -> bool) -> Result<()> {
83-
status(&open_repo(cwd)?.to_thread_local(), f)
84+
pub fn for_each_changed_file(
85+
cwd: &Path,
86+
insecure: bool,
87+
f: impl Fn(Result<FileChange>) -> bool,
88+
) -> Result<()> {
89+
status(&open_repo(cwd, insecure)?.to_thread_local(), f)
8490
}
8591

86-
fn open_repo(path: &Path) -> Result<ThreadSafeRepository> {
92+
fn open_repo(path: &Path, insecure: bool) -> Result<ThreadSafeRepository> {
8793
// custom open options
88-
let mut git_open_opts_map = gix::sec::trust::Mapping::<gix::open::Options>::default();
94+
let git_open_opts_map = gix::sec::trust::Mapping::<gix::open::Options>::default();
8995

9096
// On windows various configuration options are bundled as part of the installations
9197
// This path depends on the install location of git and therefore requires some overhead to lookup
@@ -99,30 +105,24 @@ fn open_repo(path: &Path) -> Result<ThreadSafeRepository> {
99105
includes: true,
100106
git_binary: cfg!(windows),
101107
};
102-
// change options for config permissions without touching anything else
103-
git_open_opts_map.reduced = git_open_opts_map
104-
.reduced
105-
.permissions(gix::open::Permissions {
108+
109+
let opts = if let TrustStatus::Trusted = quick_query_workspace(insecure) {
110+
git_open_opts_map.full.permissions(gix::open::Permissions {
106111
config,
107-
..gix::open::Permissions::default_for_level(gix::sec::Trust::Reduced)
108-
});
109-
git_open_opts_map.full = git_open_opts_map.full.permissions(gix::open::Permissions {
110-
config,
111-
..gix::open::Permissions::default_for_level(gix::sec::Trust::Full)
112-
});
113-
114-
let open_options = gix::discover::upwards::Options {
115-
dot_git_only: true,
116-
..Default::default()
112+
..gix::open::Permissions::default_for_level(gix::sec::Trust::Full)
113+
})
114+
} else {
115+
git_open_opts_map
116+
.reduced
117+
.permissions(gix::open::Permissions {
118+
config,
119+
..gix::open::Permissions::default_for_level(gix::sec::Trust::Reduced)
120+
})
117121
};
118122

119-
let res = ThreadSafeRepository::discover_with_environment_overrides_opts(
120-
path,
121-
open_options,
122-
git_open_opts_map,
123-
)?;
123+
let (workspace, _) = find_workspace_in(path);
124124

125-
Ok(res)
125+
Ok(ThreadSafeRepository::open_opts(workspace, opts)?)
126126
}
127127

128128
/// Emulates the result of running `git status` from the command line.

helix-vcs/src/git/test.rs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ fn missing_file() {
5454
let file = temp_git.path().join("file.txt");
5555
File::create(&file).unwrap().write_all(b"foo").unwrap();
5656

57-
assert!(git::get_diff_base(&file).is_err());
57+
assert!(git::get_diff_base(&file, true).is_err());
5858
}
5959

6060
#[test]
@@ -64,7 +64,10 @@ fn unmodified_file() {
6464
let contents = b"foo".as_slice();
6565
File::create(&file).unwrap().write_all(contents).unwrap();
6666
create_commit(temp_git.path(), true);
67-
assert_eq!(git::get_diff_base(&file).unwrap(), Vec::from(contents));
67+
assert_eq!(
68+
git::get_diff_base(&file, true).unwrap(),
69+
Vec::from(contents)
70+
);
6871
}
6972

7073
#[test]
@@ -76,7 +79,10 @@ fn modified_file() {
7679
create_commit(temp_git.path(), true);
7780
File::create(&file).unwrap().write_all(b"bar").unwrap();
7881

79-
assert_eq!(git::get_diff_base(&file).unwrap(), Vec::from(contents));
82+
assert_eq!(
83+
git::get_diff_base(&file, true).unwrap(),
84+
Vec::from(contents)
85+
);
8086
}
8187

8288
/// Test that `get_file_head` does not return content for a directory.
@@ -95,7 +101,7 @@ fn directory() {
95101

96102
std::fs::remove_dir_all(&dir).unwrap();
97103
File::create(&dir).unwrap().write_all(b"bar").unwrap();
98-
assert!(git::get_diff_base(&dir).is_err());
104+
assert!(git::get_diff_base(&dir, true).is_err());
99105
}
100106

101107
/// Test that `get_diff_base` resolves symlinks so that the same diff base is
@@ -122,8 +128,8 @@ fn symlink() {
122128
symlink("file.txt", &file_link).unwrap();
123129
create_commit(temp_git.path(), true);
124130

125-
assert_eq!(git::get_diff_base(&file_link).unwrap(), contents);
126-
assert_eq!(git::get_diff_base(&file).unwrap(), contents);
131+
assert_eq!(git::get_diff_base(&file_link, true).unwrap(), contents);
132+
assert_eq!(git::get_diff_base(&file, true).unwrap(), contents);
127133
}
128134

129135
/// Test that `get_diff_base` returns content when the file is a symlink to
@@ -147,6 +153,6 @@ fn symlink_to_git_repo() {
147153
let file_link = temp_dir.path().join("file_link.txt");
148154
symlink(&file, &file_link).unwrap();
149155

150-
assert_eq!(git::get_diff_base(&file_link).unwrap(), contents);
151-
assert_eq!(git::get_diff_base(&file).unwrap(), contents);
156+
assert_eq!(git::get_diff_base(&file_link, true).unwrap(), contents);
157+
assert_eq!(git::get_diff_base(&file, true).unwrap(), contents);
152158
}

helix-vcs/src/lib.rs

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ pub struct DiffProviderRegistry {
3030
impl DiffProviderRegistry {
3131
/// Get the given file from the VCS. This provides the unedited document as a "base"
3232
/// for a diff to be created.
33-
pub fn get_diff_base(&self, file: &Path) -> Option<Vec<u8>> {
33+
pub fn get_diff_base(&self, file: &Path, insecure: bool) -> Option<Vec<u8>> {
3434
self.providers
3535
.iter()
36-
.find_map(|provider| match provider.get_diff_base(file) {
36+
.find_map(|provider| match provider.get_diff_base(file, insecure) {
3737
Ok(res) => Some(res),
3838
Err(err) => {
3939
log::debug!("{err:#?}");
@@ -44,31 +44,36 @@ impl DiffProviderRegistry {
4444
}
4545

4646
/// Get the current name of the current [HEAD](https://stackoverflow.com/questions/2304087/what-is-head-in-git).
47-
pub fn get_current_head_name(&self, file: &Path) -> Option<Arc<ArcSwap<Box<str>>>> {
48-
self.providers
49-
.iter()
50-
.find_map(|provider| match provider.get_current_head_name(file) {
47+
pub fn get_current_head_name(
48+
&self,
49+
file: &Path,
50+
insecure: bool,
51+
) -> Option<Arc<ArcSwap<Box<str>>>> {
52+
self.providers.iter().find_map(|provider| {
53+
match provider.get_current_head_name(file, insecure) {
5154
Ok(res) => Some(res),
5255
Err(err) => {
5356
log::debug!("{err:#?}");
5457
log::debug!("failed to obtain current head name for {}", file.display());
5558
None
5659
}
57-
})
60+
}
61+
})
5862
}
5963

6064
/// Fire-and-forget changed file iteration. Runs everything in a background task. Keeps
6165
/// iteration until `on_change` returns `false`.
6266
pub fn for_each_changed_file(
6367
self,
6468
cwd: PathBuf,
69+
insecure: bool,
6570
f: impl Fn(Result<FileChange>) -> bool + Send + 'static,
6671
) {
6772
tokio::task::spawn_blocking(move || {
6873
if self
6974
.providers
7075
.iter()
71-
.find_map(|provider| provider.for_each_changed_file(&cwd, &f).ok())
76+
.find_map(|provider| provider.for_each_changed_file(&cwd, insecure, &f).ok())
7277
.is_none()
7378
{
7479
f(Err(anyhow!("no diff provider returns success")));
@@ -102,30 +107,31 @@ enum DiffProvider {
102107
}
103108

104109
impl DiffProvider {
105-
fn get_diff_base(&self, file: &Path) -> Result<Vec<u8>> {
110+
fn get_diff_base(&self, file: &Path, insecure: bool) -> Result<Vec<u8>> {
106111
match self {
107112
#[cfg(feature = "git")]
108-
Self::Git => git::get_diff_base(file),
113+
Self::Git => git::get_diff_base(file, insecure),
109114
Self::None => bail!("No diff support compiled in"),
110115
}
111116
}
112117

113-
fn get_current_head_name(&self, file: &Path) -> Result<Arc<ArcSwap<Box<str>>>> {
118+
fn get_current_head_name(&self, file: &Path, insecure: bool) -> Result<Arc<ArcSwap<Box<str>>>> {
114119
match self {
115120
#[cfg(feature = "git")]
116-
Self::Git => git::get_current_head_name(file),
121+
Self::Git => git::get_current_head_name(file, insecure),
117122
Self::None => bail!("No diff support compiled in"),
118123
}
119124
}
120125

121126
fn for_each_changed_file(
122127
&self,
123128
cwd: &Path,
129+
insecure: bool,
124130
f: impl Fn(Result<FileChange>) -> bool,
125131
) -> Result<()> {
126132
match self {
127133
#[cfg(feature = "git")]
128-
Self::Git => git::for_each_changed_file(cwd, f),
134+
Self::Git => git::for_each_changed_file(cwd, insecure, f),
129135
Self::None => bail!("No diff support compiled in"),
130136
}
131137
}

helix-view/src/document.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,12 +1297,13 @@ impl Document {
12971297
self.pickup_last_saved_time();
12981298
self.detect_indent_and_line_ending();
12991299

1300-
match provider_registry.get_diff_base(&path) {
1300+
let insecure = self.config.load().insecure;
1301+
match provider_registry.get_diff_base(&path, insecure) {
13011302
Some(diff_base) => self.set_diff_base(diff_base),
13021303
None => self.diff_handle = None,
13031304
}
13041305

1305-
self.version_control_head = provider_registry.get_current_head_name(&path);
1306+
self.version_control_head = provider_registry.get_current_head_name(&path, insecure);
13061307

13071308
Ok(())
13081309
}

helix-view/src/editor.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1945,10 +1945,13 @@ impl Editor {
19451945
Editor::doc_diagnostics(&self.language_servers, &self.diagnostics, &doc);
19461946
doc.replace_diagnostics(diagnostics, &[], None);
19471947

1948-
if let Some(diff_base) = self.diff_providers.get_diff_base(&path) {
1948+
let insecure = self.config().insecure;
1949+
if let Some(diff_base) = self.diff_providers.get_diff_base(&path, insecure) {
19491950
doc.set_diff_base(diff_base);
19501951
}
1951-
doc.set_version_control_head(self.diff_providers.get_current_head_name(&path));
1952+
doc.set_version_control_head(
1953+
self.diff_providers.get_current_head_name(&path, insecure),
1954+
);
19521955

19531956
let id = self.new_document(doc);
19541957
self.launch_language_servers(id);

0 commit comments

Comments
 (0)