Skip to content

Commit 136eb37

Browse files
committed
git-sec: adopt git-for-windows exception rules
1 parent 5ac8a3b commit 136eb37

File tree

5 files changed

+38
-17
lines changed

5 files changed

+38
-17
lines changed

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.

git-discover/tests/upwards/mod.rs

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,7 @@ use std::path::PathBuf;
33
use git_discover::repository::Kind;
44

55
fn expected_trust() -> git_sec::Trust {
6-
#[cfg(not(windows))]
7-
{
8-
git_sec::Trust::Full
9-
}
10-
#[cfg(windows)]
11-
{
12-
if is_ci::cached() {
13-
git_sec::Trust::Reduced
14-
} else {
15-
git_sec::Trust::Full
16-
}
17-
}
6+
git_sec::Trust::Full
187
}
198

209
mod ceiling_dirs;

git-sec/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ thiserror = { version = "1.0.26", optional = true }
2626
libc = "0.2.123"
2727

2828
[target.'cfg(windows)'.dependencies]
29+
dirs = "4"
2930
windows = { version = "0.36.0", features = [ "alloc",
3031
"Win32_Foundation",
3132
"Win32_Security_Authorization",

git-sec/src/identity.rs

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,16 @@ mod impl_ {
5151
use windows::{
5252
core::{Error, PCWSTR},
5353
Win32::{
54-
Foundation::{HANDLE, PSID},
54+
Foundation::{CloseHandle, BOOL, HANDLE, PSID},
5555
Security::{
5656
Authorization::{GetNamedSecurityInfoW, SE_FILE_OBJECT},
57-
EqualSid, GetTokenInformation, TokenOwner, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR,
58-
TOKEN_OWNER, TOKEN_QUERY,
57+
CheckTokenMembership, EqualSid, GetTokenInformation, IsWellKnownSid, TokenOwner,
58+
WinBuiltinAdministratorsSid, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, TOKEN_OWNER,
59+
TOKEN_QUERY,
5960
},
6061
System::{
6162
Memory::LocalFree,
62-
Threading::{GetCurrentProcess, OpenProcessToken},
63+
Threading::{GetCurrentProcess, GetCurrentThread, OpenProcessToken, OpenThreadToken},
6364
},
6465
},
6566
};
@@ -68,6 +69,13 @@ mod impl_ {
6869
let mut is_owned = false;
6970
let path = path.as_ref();
7071

72+
// Home is not actually owned by the corresponding user
73+
// but it can be considered de-facto owned by the user
74+
// Ignore errors here and just do the regular checks below
75+
if Some(path) == dirs::home_dir().as_deref() {
76+
return Ok(true);
77+
}
78+
7179
#[allow(unsafe_code)]
7280
unsafe {
7381
let mut folder_owner = PSID::default();
@@ -86,7 +94,10 @@ mod impl_ {
8694
// Workaround for https://github.com/microsoft/win32metadata/issues/884
8795
if result.is_ok() {
8896
let mut token = HANDLE::default();
89-
OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token).ok()?;
97+
// Use the current thread token if possible, otherwise open the process token
98+
OpenThreadToken(GetCurrentThread(), TOKEN_QUERY, true, &mut token)
99+
.ok()
100+
.or_else(|_| OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token).ok())?;
90101

91102
let mut buffer_size = 0;
92103
let mut buffer = Vec::<u8>::new();
@@ -106,6 +117,16 @@ mod impl_ {
106117
let token_owner = (*token_owner).Owner;
107118

108119
is_owned = EqualSid(folder_owner, token_owner).as_bool();
120+
121+
// Admin-group owned folders are considered owned by the current user, if they are in the admin group
122+
if !is_owned && IsWellKnownSid(token_owner, WinBuiltinAdministratorsSid).as_bool() {
123+
let mut is_member = BOOL::default();
124+
// TODO: re-use the handle
125+
match CheckTokenMembership(HANDLE::default(), token_owner, &mut is_member).ok() {
126+
Err(e) => err_msg = Some(format!("Couldn't check if user is an administrator: {}", e)),
127+
Ok(()) => is_owned = is_member.as_bool(),
128+
}
129+
}
109130
} else {
110131
err_msg = format!(
111132
"Couldn't get actual token information for current process with err: {}",
@@ -120,6 +141,7 @@ mod impl_ {
120141
)
121142
.into();
122143
}
144+
CloseHandle(token);
123145
} else {
124146
err_msg = format!(
125147
"Couldn't get security information for path '{}' with err {}",

git-sec/tests/identity/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,11 @@ fn is_path_owned_by_current_user() -> crate::Result {
77
assert!(git_sec::identity::is_path_owned_by_current_user(dir.path())?);
88
Ok(())
99
}
10+
11+
#[test]
12+
#[cfg(windows)]
13+
fn windows_home() -> crate::Result {
14+
let home = dirs::home_dir().expect("home dir is available");
15+
assert!(git_sec::identity::is_path_owned_by_current_user(home)?);
16+
Ok(())
17+
}

0 commit comments

Comments
 (0)