Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion gix-sec/src/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ mod impl_ {

pub fn is_path_owned_by_current_user(path: &Path) -> io::Result<bool> {
use windows_sys::Win32::{
Foundation::{GetLastError, LocalFree, ERROR_INSUFFICIENT_BUFFER, ERROR_SUCCESS},
Foundation::{GetLastError, LocalFree, ERROR_INSUFFICIENT_BUFFER, ERROR_INVALID_FUNCTION, ERROR_SUCCESS},
Security::{
Authorization::{GetNamedSecurityInfoW, SE_FILE_OBJECT},
CheckTokenMembership, EqualSid, GetTokenInformation, IsWellKnownSid, TokenOwner,
Expand Down Expand Up @@ -123,6 +123,13 @@ mod impl_ {
);

if result != ERROR_SUCCESS {
// On some mounted drives (like network drives), GetNamedSecurityInfoW
// may fail with ERROR_INVALID_FUNCTION. In such cases, we cannot determine
// ownership, so we default to reduced trust (false) rather than failing completely.
if result == ERROR_INVALID_FUNCTION {
return Ok(false);
}

let inner = io::Error::from_raw_os_error(result as _);
error!(
inner,
Expand Down
26 changes: 26 additions & 0 deletions gix-sec/tests/sec/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,36 @@ fn is_path_owned_by_current_user() -> crate::Result {
Ok(())
}

#[test]
fn is_path_owned_by_current_user_nonexistent() {
let nonexistent = std::path::Path::new("/this/path/does/not/exist");
let result = gix_sec::identity::is_path_owned_by_current_user(nonexistent);
assert!(result.is_err(), "Should fail for nonexistent paths");
assert_eq!(result.unwrap_err().kind(), std::io::ErrorKind::NotFound);
}

#[test]
#[cfg(windows)]
fn windows_home() -> crate::Result {
let home = gix_path::env::home_dir().expect("home dir is available");
assert!(gix_sec::identity::is_path_owned_by_current_user(&home)?);
Ok(())
}

#[test]
fn test_trust_behavior_scenarios() {
// Test 1: Current directory (should work)
let current_dir = std::env::current_dir().unwrap();
println!("Testing current directory: {:?}", current_dir);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Don't println in tests, use assertions with third arguments instead, like assert_eq!(a, b, "format {} string").

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 0d664ac. Replaced all println statements with proper assertions using third arguments for better error messages.

match gix_sec::identity::is_path_owned_by_current_user(&current_dir) {
Ok(owned) => println!(" Result: owned = {}", owned),
Err(e) => println!(" Error: {}", e),
}

// Test 3: Trust from path ownership
println!("Testing trust derivation from path ownership:");
match gix_sec::Trust::from_path_ownership(&current_dir) {
Ok(trust) => println!(" Trust level: {:?}", trust),
Err(e) => println!(" Error: {}", e),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Remove this, makes no sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the redundant Trust::from_path_ownership test in commit b2dca1b. The test was duplicating functionality already covered by the is_path_owned_by_current_user test above it.

}
}
Loading