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

Fix clone within a directory whose parent isn't readable by the current user #2533

Merged
merged 1 commit into from
Mar 5, 2020

Conversation

dscho
Copy link
Member

@dscho dscho commented Mar 2, 2020

Even if the patch looks like it has nothing to do with cloning (or for that matter, with creating leading directories), this fixes #2531.

When creating directories via `safe_create_leading_directories()`, we
might encounter an already-existing directory which is not
readable by the current user. To handle that situation, Git's code calls
`stat()` to determine whether we're looking at a directory.

In such a case, `CreateFile()` will fail, though, no matter what, and
consequently `mingw_stat()` will fail, too. But POSIX semantics seem to
still allow `stat()` to go forward.

So let's call `mingw_lstat()` for the rescue if we fail to get a file
handle due to denied permission in `mingw_stat()`, and fill the stat
info that way.

We need to be careful to not allow this to go forward in case that we're
looking at a symbolic link: to resolve the link, we would still have to
create a file handle, and we just found out that we cannot. Therefore,
`stat()` still needs to fail with `EACCES` in that case.

This fixes git-for-windows#2531.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member Author

dscho commented Mar 4, 2020

@danthe1st @byNoobiYT could you test this?

@dscho
Copy link
Member Author

dscho commented Mar 5, 2020

Oh well, I guess I'll merge it before v2.26.0-rc0 (which should have happened, but got delayed for some reason).

@dscho dscho merged commit 0917453 into git-for-windows:master Mar 5, 2020
@dscho dscho added this to the Next release milestone Mar 5, 2020
@dscho dscho deleted the try-stat-harder branch March 5, 2020 09:45
git-for-windows-ci pushed a commit that referenced this pull request Mar 5, 2020
Fix clone within a directory whose parent isn't readable by the current user
git-for-windows-ci pushed a commit that referenced this pull request Mar 5, 2020
Fix clone within a directory whose parent isn't readable by the current user
@danthe1st
Copy link

Should we still test it? If so, is it possible to test it without Visual Studio?

@dscho
Copy link
Member Author

dscho commented Mar 5, 2020

@danthe1st yes, please test it.

You won't be able to compile it with Visual Studio unless you get the entire Git for Windows SDK, which far outweighs Visual Studio.

But we do have snapshot builds for master.

Unfortunately, the build failed, but the next build hopefully won't, and you should be able to test the latest snapshot.

@dscho
Copy link
Member Author

dscho commented Mar 5, 2020

dscho added a commit to git-for-windows/build-extra that referenced this pull request Mar 5, 2020
Git for Windows [can now clone into directories the current user
can write to, even if they lack permission to even read the parent
directory](git-for-windows/git#2533).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@danthe1st
Copy link

I tested it with that snapshot and I get a different error (with a better error message) prompting me for username and password. When entering the credentials correctly, I get the following error:

C:\parent\child>C:\PRGM\git-snapshot\bin\git.exe clone https://github.com/octocat/HelloWorld.git
Cloning into 'HelloWorld'...
fatal: UnauthorizedAccessException encountered.
Der Zugriff auf den Pfad "C:\parent" wurde verweigert.
Username for 'https://github.com': danthe1st
Password for 'https://danthe1st@github.com':
remote: Repository not found.
fatal: repository 'https://github.com/octocat/HelloWorld.git/' not found

(Der Zugriff auf den Pfad "C:\parent" wurde verweigert. means Access denied for path "C:\parent")

If I enter incorrect credentials, I get a normal Authentication failed error.

@dscho
Copy link
Member Author

dscho commented Mar 5, 2020

@danthe1st hmm. Can you build Git for Windows from source? If so, could you start the command in the debugger and set a breakpoint at the location changed by this PR and investigate further?

git-for-windows-ci pushed a commit that referenced this pull request Mar 5, 2020
Fix clone within a directory whose parent isn't readable by the current user
git-for-windows-ci pushed a commit that referenced this pull request Mar 5, 2020
Fix clone within a directory whose parent isn't readable by the current user
dscho added a commit that referenced this pull request Mar 6, 2020
Fix clone within a directory whose parent isn't readable by the current user
@danthe1st
Copy link

I got a Bluescreen(actually a greenscreen) while compiling git for windows(master for now) saying SYSTEM_SERVICE_EXCEPTION. I will try to do it again...

@danthe1st
Copy link

danthe1st commented Mar 6, 2020

I successfully compiled it but when cloning the repository(not even in the parent/child directory), I get the following error:

$ ./git.exe clone https://github.com/octocat/Hello-World.git
Cloning into 'Hello-World'...
warning: templates not found in C:/PRGM/git-sdk/usr/src/share/git-core/templates
git: 'remote-https' ist kein Git-Befehl. Siehe 'git --help'.

(ist kein Git-Befehl means is not a git command)
It seems like it might take a while until I get git from source to work.

@danthe1st
Copy link

danthe1st commented Mar 6, 2020

After adding the git directory to the PATH manually, it works in the SDK git bash:

main user@hostname MINGW64 /c/parent/child
$ git clone https://github.com/octocat/Hello-World.git
Cloning into 'Hello-World'...
warning: templates not found in C:/PRGM/git-sdk/usr/src/share/git-core/templates
git: 'credential-manager' is not a git command. See 'git --help'.
git: 'credential-manager' is not a git command. See 'git --help'.
remote: Enumerating objects: 13, done.
remote: Total 13 (delta 0), reused 0 (delta 0), pack-reused 13
Unpacking objects: 100% (13/13), 1.53 KiB | 7.00 KiB/s, done.

However, I don't get it to work in the normal cmd because of missing DLLs.

@dscho
Copy link
Member Author

dscho commented Mar 6, 2020

Hmm. That's strange.

As to running in-place, yes, the common way is to use bin-wrappers/git [...].

@dscho
Copy link
Member Author

dscho commented Mar 6, 2020

git: 'credential-manager' is not a git command. See 'git --help'.

Oh! I think I could imagine that Git Credential-Manager might be the culprit. Because...

fatal: UnauthorizedAccessException encountered.
Der Zugriff auf den Pfad "C:\parent" wurde verweigert.
Username for 'https://github.com': danthe1st
Password for 'https://danthe1st@github.com':
remote: Repository not found.

This seems to not be coming from Git directly, but from the Git Credential Manager! (I was wondering about that exception, we do not use exceptions in core Git...)

Maybe you can try again in the SDK, after appending /mingw64/libexec/git-core to the PATH?

@danthe1st
Copy link

danthe1st commented Mar 6, 2020

It does work (in the SDK) even if credential-manager is not there.

@dscho
Copy link
Member Author

dscho commented Mar 6, 2020

It does work even if credential-manager is not there.

What I meant to say, I think that you see the failures because git-credential-manager.exe is configured.

In other words, if you run git -c credential.helper= clone https://github.com/octocat/Hello-World.git, it should work.

However, I am really curious now: why does that Credential Helper come into play, anyway? It's not like the Hello-World.git repository is a private one. It does not need you to authenticate.

Strange. Really strange.

@danthe1st
Copy link

danthe1st commented Mar 6, 2020

Yes, it does work with the option without a warning. But it didn't fail before (without the option), it was just showing the error.

@danthe1st
Copy link

danthe1st commented Mar 6, 2020

But note that it worked when I compiled it from master(commit 091745393cb11db6eb55eab509d3d0a1a243c7d5) but it didn't work with the snapshot (git version 2.25.1.windows.1.7.g091745393c) that seems to be the same version.

@dscho
Copy link
Member Author

dscho commented Mar 6, 2020

The difference is that the snapshot version configures credential.helper in the system config. The SDK does not.

@danthe1st
Copy link

Yes, it works without the credential.helper in the snapshot version:

C:\parent\child>git -c credential.helper= clone https://github.com/octocat/Hello-World.git
Cloning into 'Hello-World'...
remote: Enumerating objects: 13, done.
remote: Total 13 (delta 0), reused 0 (delta 0), pack-reused 13
Unpacking objects: 100% (13/13), 1.53 KiB | 9.00 KiB/s, done.

@dscho
Copy link
Member Author

dscho commented Mar 6, 2020

Okay, so our work here is done, eh? :-)

git-for-windows-ci pushed a commit that referenced this pull request Mar 6, 2020
Fix clone within a directory whose parent isn't readable by the current user
git-for-windows-ci pushed a commit that referenced this pull request Mar 9, 2020
Fix clone within a directory whose parent isn't readable by the current user
git-for-windows-ci pushed a commit that referenced this pull request Mar 9, 2020
Fix clone within a directory whose parent isn't readable by the current user
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to create HEAD.lock when cloning inside directory with permissions in directory without any permissions
2 participants