Skip to content

Commit 1df379a

Browse files
committed
change!: remove Permissions::git_dir field entirely.
It was meant to help dealing with bailing out if the git dir isn't fully trusted, but the way this was done was over-engineered especially since the read-only permission level wasn't implemented at all. That function is now performed by a new flag, the `bail_on_untrusted` which is off by default.
1 parent be6114e commit 1df379a

File tree

3 files changed

+19
-17
lines changed

3 files changed

+19
-17
lines changed

git-repository/src/open.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ pub struct Options {
6868
pub(crate) git_dir_trust: Option<git_sec::Trust>,
6969
pub(crate) filter_config_section: Option<fn(&git_config::file::Metadata) -> bool>,
7070
pub(crate) lossy_config: Option<bool>,
71+
pub(crate) bail_if_untrusted: bool,
7172
}
7273

7374
#[derive(Default, Clone)]
@@ -118,7 +119,6 @@ impl Options {
118119
git_prefix: deny,
119120
}
120121
},
121-
..Permissions::default()
122122
})
123123
}
124124
}
@@ -166,6 +166,17 @@ impl Options {
166166
self
167167
}
168168

169+
/// If true, default false, and if the repository's trust level is not `Full`
170+
/// (see [`with()`][Self::with()] for more), then the open operation will fail.
171+
///
172+
/// Use this to mimic `git`s way of handling untrusted repositories. Note that `gitoxide` solves
173+
/// this by not using configuration from untrusted sources and by generally being secured against
174+
/// doctored input files which at worst could cause out-of-memory at the time of writing.
175+
pub fn bail_if_untrusted(mut self, toggle: bool) -> Self {
176+
self.bail_if_untrusted = toggle;
177+
self
178+
}
179+
169180
/// Set the filter which determines if a configuration section can be used to read values from,
170181
/// hence it returns true if it is eligible.
171182
///
@@ -201,13 +212,15 @@ impl git_sec::trust::DefaultForLevel for Options {
201212
git_dir_trust: git_sec::Trust::Full.into(),
202213
filter_config_section: Some(config::section::is_trusted),
203214
lossy_config: None,
215+
bail_if_untrusted: false,
204216
},
205217
git_sec::Trust::Reduced => Options {
206218
object_store_slots: git_odb::store::init::Slots::Given(32), // limit resource usage
207219
replacement_objects: ReplacementObjects::Disable, // don't be tricked into seeing manufactured objects
208220
permissions: Permissions::default_for_level(level),
209221
git_dir_trust: git_sec::Trust::Reduced.into(),
210222
filter_config_section: Some(config::section::is_trusted),
223+
bail_if_untrusted: false,
211224
lossy_config: None,
212225
},
213226
}
@@ -301,12 +314,8 @@ impl ThreadSafeRepository {
301314
filter_config_section,
302315
ref replacement_objects,
303316
lossy_config,
304-
permissions:
305-
Permissions {
306-
git_dir: ref git_dir_perm,
307-
ref env,
308-
config,
309-
},
317+
bail_if_untrusted,
318+
permissions: Permissions { ref env, config },
310319
} = options;
311320
let git_dir_trust = git_dir_trust.expect("trust must be been determined by now");
312321

@@ -344,7 +353,7 @@ impl ThreadSafeRepository {
344353
config,
345354
)?;
346355

347-
if **git_dir_perm != git_sec::ReadWrite::all() {
356+
if bail_if_untrusted && git_dir_trust != git_sec::Trust::Full {
348357
check_safe_directories(&git_dir, git_install_dir.as_deref(), home.as_deref(), &config)?;
349358
}
350359

git-repository/src/repository/permissions.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
1-
use git_sec::{permission::Resource, Access, Trust};
1+
use git_sec::{Access, Trust};
22

33
use crate::permission;
44

55
/// Permissions associated with various resources of a git repository
66
#[derive(Debug, Clone)]
77
pub struct Permissions {
8-
/// Control how a git-dir can be used.
9-
///
10-
/// Note that a repository won't be usable at all unless read and write permissions are given.
11-
pub git_dir: Access<Resource, git_sec::ReadWrite>,
128
/// Permissions related to the environment
139
pub env: Environment,
1410
/// Permissions related to the handling of git configuration.
@@ -90,7 +86,6 @@ impl Permissions {
9086
/// thus refusing all operations in it.
9187
pub fn strict() -> Self {
9288
Permissions {
93-
git_dir: Access::resource(git_sec::ReadWrite::READ),
9489
env: Environment::all(),
9590
config: Config::all(),
9691
}
@@ -103,7 +98,6 @@ impl Permissions {
10398
/// anything else that could cause us to write into unknown locations or use programs beyond our `PATH`.
10499
pub fn secure() -> Self {
105100
Permissions {
106-
git_dir: Access::resource(git_sec::ReadWrite::all()),
107101
env: Environment::all(),
108102
config: Config::all(),
109103
}
@@ -113,7 +107,6 @@ impl Permissions {
113107
/// does with owned repositories.
114108
pub fn all() -> Self {
115109
Permissions {
116-
git_dir: Access::resource(git_sec::ReadWrite::all()),
117110
env: Environment::all(),
118111
config: Config::all(),
119112
}

git-repository/src/repository/snapshots.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ impl crate::Repository {
101101
match path.interpolate(git_config::path::interpolate::Context {
102102
git_install_dir: Some(install_dir.as_path()),
103103
home_dir: home.as_deref(),
104-
home_for_user: if self.options.permissions.git_dir.is_all() {
104+
home_for_user: if self.options.git_dir_trust.expect("trust is set") == git_sec::Trust::Full {
105105
Some(git_config::path::interpolate::home_for_user)
106106
} else {
107107
None

0 commit comments

Comments
 (0)