Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mingw: special-case administrators even more #1893

Closed
wants to merge 2 commits into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Mar 25, 2025

On Windows, a file created by a process running in elevated mode is owned by the Administrators group (not by the user's account who would otherwise be able to modify or delete the file in non-elevated mode). Let's adjust the "safe directory" feature accordingly.

Naturally, this patch series does not add a regression test (because it can't, you cannot automate elevating processes).

This patch series is a companion of microsoft#712.

dscho added 2 commits March 25, 2025 10:23
The check for dubious ownership has one particular quirk on Windows: if
running as an administrator, files owned by the Administrators _group_
are considered owned by the user.

The rationale for that is: When running in elevated mode, Git creates
files that aren't owned by the individual user but by the Administrators
group.

There is yet another quirk, though: The check I introduced to determine
whether the current user is an administrator uses the
`CheckTokenMembership()` function with the current process token. And
that check only succeeds when running in elevated mode!

Let's be a bit more lenient here and look harder whether the current
user is an administrator. We do this by looking for a so-called "linked
token". That token exists when administrators run in non-elevated mode,
and can be used to create a new process in elevated mode. And feeding
_that_ token to the `CheckTokenMembership()` function succeeds!

Signed-off-by: Johannes Schindelin <[email protected]>
This adds a new sub-sub-command for `test-tool`, simply passing through
the command-line arguments to the `is_path_owned_by_current_user()`
function.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member Author

dscho commented Mar 25, 2025

/submit

Copy link

gitgitgadget bot commented Mar 25, 2025

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1893/dscho/admins-are-admins-on-windows-v1

To fetch this version to local tag pr-1893/dscho/admins-are-admins-on-windows-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1893/dscho/admins-are-admins-on-windows-v1

Copy link

gitgitgadget bot commented Mar 25, 2025

This patch series was integrated into seen via git@5a79ced.

@gitgitgadget gitgitgadget bot added the seen label Mar 25, 2025
Copy link

gitgitgadget bot commented Mar 28, 2025

This branch is now known as js/mingw-admins-are-special.

Copy link

gitgitgadget bot commented Mar 28, 2025

This patch series was integrated into seen via git@c2a7c69.

Copy link

gitgitgadget bot commented Mar 28, 2025

This patch series was integrated into next via git@dfcb966.

@gitgitgadget gitgitgadget bot added the next label Mar 28, 2025
Copy link

gitgitgadget bot commented Mar 29, 2025

This patch series was integrated into seen via git@23d2b41.

Copy link

gitgitgadget bot commented Apr 7, 2025

There was a status update in the "Cooking" section about the branch js/mingw-admins-are-special on the Git mailing list:

"Dubious ownership" checks on Windows has been tightened up.

Will merge to 'master'.
source: <[email protected]>

Copy link

gitgitgadget bot commented Apr 7, 2025

This patch series was integrated into seen via git@45e31f0.

Copy link

gitgitgadget bot commented Apr 7, 2025

This patch series was integrated into master via git@45e31f0.

Copy link

gitgitgadget bot commented Apr 7, 2025

This patch series was integrated into next via git@45e31f0.

@gitgitgadget gitgitgadget bot added the master label Apr 7, 2025
Copy link

gitgitgadget bot commented Apr 7, 2025

Closed via 45e31f0.

@gitgitgadget gitgitgadget bot closed this Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant