Skip to content

Commit c9ebac7

Browse files
codexByron
andcommitted
feat!: allow assuming trust during upward discovery
upward discovery options should allow callers to override the trust level during discovery. When a trust level is provided, discovery should not compute trust from path ownership and should instead assume the provided level. This intentionally changes the public Options::trust (formerly `required_trust`) field from gix_sec::Trust to gix_discover::upwards::Trust so the field can represent either a required computed trust threshold or an assumed trust level. Existing callers that set required_trust directly need to wrap their threshold in Trust::Required(_). The fix keeps the default as the existing reduced-trust requirement, while Trust::Assume(level) skips Trust::from_path_ownership() and returns the provided trust level. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
1 parent 5dfb44d commit c9ebac7

3 files changed

Lines changed: 66 additions & 16 deletions

File tree

gix-discover/src/upwards/mod.rs

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
mod types;
2-
pub use types::{Error, Options};
2+
pub use types::{Error, Options, TrustPolicy};
33

44
mod util;
55

@@ -8,7 +8,7 @@ pub(crate) mod function {
88

99
use gix_sec::Trust;
1010

11-
use super::{Error, Options};
11+
use super::{Error, Options, TrustPolicy};
1212
#[cfg(unix)]
1313
use crate::upwards::util::device_id;
1414
use crate::{
@@ -27,14 +27,14 @@ pub(crate) mod function {
2727
pub fn discover_opts(
2828
directory: &Path,
2929
Options {
30-
required_trust,
30+
trust,
3131
ceiling_dirs,
3232
match_ceiling_dir_or_error,
3333
cross_fs,
3434
current_dir,
3535
dot_git_only,
3636
}: Options<'_>,
37-
) -> Result<(crate::repository::Path, Trust), Error> {
37+
) -> Result<(crate::repository::Path, gix_sec::Trust), Error> {
3838
// Normalize the path so that `Path::parent()` _actually_ gives
3939
// us the parent directory. (`Path::parent` just strips off the last
4040
// path component, which means it will not do what you expect when
@@ -67,9 +67,15 @@ pub(crate) mod function {
6767
.or_else(|_| dir.as_ref().strip_prefix(cwd.as_ref()))
6868
.is_ok();
6969

70-
let filter_by_trust = |x: &Path| -> Result<Option<Trust>, Error> {
71-
let trust = Trust::from_path_ownership(x).map_err(|err| Error::CheckTrust { path: x.into(), err })?;
72-
Ok((trust >= required_trust).then_some(trust))
70+
let filter_by_trust = |x: &Path| -> Result<Result<Trust, Trust>, Error> {
71+
match trust {
72+
TrustPolicy::Required(required) => {
73+
let trust =
74+
Trust::from_path_ownership(x).map_err(|err| Error::CheckTrust { path: x.into(), err })?;
75+
Ok(if trust >= required { Ok(trust) } else { Err(required) })
76+
}
77+
TrustPolicy::Assume(trust) => Ok(Ok(trust)),
78+
}
7379
};
7480

7581
let max_height = if !ceiling_dirs.is_empty() {
@@ -134,7 +140,7 @@ pub(crate) mod function {
134140
None => is_git(&cursor),
135141
} {
136142
match filter_by_trust(&cursor)? {
137-
Some(trust) => {
143+
Ok(trust) => {
138144
// TODO: test this more, it definitely doesn't always find the shortest path to a directory
139145
let path = if dir_made_absolute {
140146
shorten_path_with_cwd(cursor, cwd.as_ref())
@@ -150,11 +156,11 @@ pub(crate) mod function {
150156
trust,
151157
));
152158
}
153-
None => {
159+
Err(required) => {
154160
break 'outer Err(Error::NoTrustedGitRepository {
155161
path: dir.into_owned(),
156162
candidate: cursor,
157-
required: required_trust,
163+
required,
158164
});
159165
}
160166
}
@@ -198,7 +204,7 @@ pub(crate) mod function {
198204
/// the trust level derived from Path ownership.
199205
///
200206
/// Fail if no valid-looking git repository could be found.
201-
pub fn discover(directory: &Path) -> Result<(crate::repository::Path, Trust), Error> {
207+
pub fn discover(directory: &Path) -> Result<(crate::repository::Path, gix_sec::Trust), Error> {
202208
discover_opts(directory, Default::default())
203209
}
204210
}

gix-discover/src/upwards/types.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,30 @@ pub enum Error {
3232
},
3333
}
3434

35+
/// How to obtain the trust level for a discovered repository.
36+
#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
37+
pub enum TrustPolicy {
38+
/// Determine trust from repository ownership and require it to be at least the given level.
39+
Required(gix_sec::Trust),
40+
/// Trust computation is skipped and the given trust level is assumed.
41+
Assume(gix_sec::Trust),
42+
}
43+
44+
impl Default for TrustPolicy {
45+
fn default() -> Self {
46+
TrustPolicy::Required(gix_sec::Trust::Reduced)
47+
}
48+
}
49+
3550
/// Options to help guide the [discovery][crate::upwards()] of repositories, along with their options
3651
/// when instantiated.
3752
pub struct Options<'a> {
38-
/// When discovering a repository, assure it has at least this trust level or ignore it otherwise.
53+
/// When discovering a repository, determine how trust should be obtained.
3954
///
40-
/// This defaults to [`Reduced`][gix_sec::Trust::Reduced] as our default settings are geared towards avoiding abuse.
41-
/// Set it to `Full` to only see repositories that [are owned by the current user][gix_sec::Trust::from_path_ownership()].
42-
pub required_trust: gix_sec::Trust,
55+
/// This defaults to [`Required(Reduced)`][TrustPolicy::Required] as our default settings are geared towards avoiding abuse.
56+
/// Set it to `Required(Full)` to only see repositories that [are owned by the current user][gix_sec::Trust::from_path_ownership()],
57+
/// or [`TrustPolicy::Assume`] to skip trust computation and return the given trust level.
58+
pub trust: TrustPolicy,
4359
/// When discovering a repository, ignore any repositories that are located in these directories or any of their parents.
4460
///
4561
/// Note that we ignore ceiling directories if the search directory is directly on top of one, which by default is an error
@@ -73,7 +89,7 @@ pub struct Options<'a> {
7389
impl Default for Options<'_> {
7490
fn default() -> Self {
7591
Options {
76-
required_trust: gix_sec::Trust::Reduced,
92+
trust: TrustPolicy::default(),
7793
ceiling_dirs: vec![],
7894
match_ceiling_dir_or_error: true,
7995
cross_fs: false,

gix-discover/tests/discover/upwards/mod.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,34 @@ fn expected_trust() -> gix_sec::Trust {
1212

1313
mod ceiling_dirs;
1414

15+
#[test]
16+
fn can_override_computed_trust() -> crate::Result {
17+
let dir = repo_path()?.join("some/very/deeply/nested/subdir");
18+
let overridden_trust = match expected_trust() {
19+
gix_sec::Trust::Full => gix_sec::Trust::Reduced,
20+
gix_sec::Trust::Reduced => gix_sec::Trust::Full,
21+
};
22+
23+
let (path, trust) = gix_discover::upwards_opts(
24+
&dir,
25+
gix_discover::upwards::Options {
26+
trust: gix_discover::upwards::TrustPolicy::Assume(overridden_trust),
27+
..Default::default()
28+
},
29+
)?;
30+
31+
assert_eq!(
32+
path.kind(),
33+
Kind::WorkTree { linked_git_dir: None },
34+
"discovery still finds the worktree"
35+
);
36+
assert_eq!(
37+
trust, overridden_trust,
38+
"the caller-provided trust is returned instead of the computed ownership trust"
39+
);
40+
Ok(())
41+
}
42+
1543
#[test]
1644
fn from_bare_git_dir() -> crate::Result {
1745
let dir = repo_path()?.join("bare.git");

0 commit comments

Comments
 (0)