safe.directory handling broken when using Cygwin Git

[me-and]

The fix at f25a3a9 is great for the vast majority of folk, but the way safe.directory is set and handled breaks my use case: I'm using Cygwin Git on Windows runners (I'm the maintainer of a number of Cygwin packages, including the Cygwin Git package). I'm installing Cygwin Git on the runner and adding it to the PATH before calling actions/checkout, which means actions/checkout uses Cygwin Git rather than the default Git that's present on the runner.

Prior to Git v2.35.1, this worked perfectly. Using Git v2.35.1 or v2.35.2 with actions/checkout@v2.4.0, I needed to configure safe.directory before calling actions/checkout, but that makes sense and worked. However, with actions/checkout@v2.4.1, safe.directory gets overridden to (say) D:\a\Cygwin-Git\Cygwin-Git, and that doesn't work, because that's not the sort of path Cygwin Git expects. Even setting safe.directory to /cygdrive/d/a/Cygwin-Git/Cygwin-Git in an earlier step doesn't work, because when git gets called as part of actions/checkout, it doesn't look at the default global .gitconfig at all.

I'm not sure what the best solution is here; I'm aware my use case is fairly niche. Options I can see:

• Don't use actions/checkout, or at least maintain my own fork of actions/checkout that handles Cygwin gracefully. This'd be a shame, but I entirely understand that supporting non-default Git installations on the runners isn't what y'all signed up for.
• I work out how to get Cygwin Git to accept Windows-style paths to be handled in this configuration option, as well as the more POSIX-like Cygwin ones. There's already code in the Cygwin wrapper layers that handles this when the executable is called – which is why the call to git init works at all – but that layer is bypassed after the initial executable call.
• Set safe.directory to * rather than a specific path in this action. I think that's safe – certainly I can't immediately come up with something that would break, given the config would only exist for the duration of the action run – but it might not be in the spirit of that configuration option.
• Add some configuration toggle to make this scenario work; default to the behaviour added in f25a3a9, but allowing someone using actions/checkout to disable the new behaviour.
• Something else cunning I haven't yet thought of.

For reference, the output I'm getting right now when trying to use actions/checkout looks like this:

Run actions/checkout@v2
Syncing repository: me-and/Cygwin-Git
Getting Git version info
Temporarily overriding HOME='D:\a\_temp\01a36204-8528-42[13](https://github.com/me-and/Cygwin-Git/runs/6030136677?check_suite_focus=true#step:4:13)-acd7-fa55cd902b9e' before making global git config changes
Adding working directory to the temporary git global config as a safe directory
C:\cygwin\bin\git.exe config --global --add safe.directory D:\a\Cygwin-Git\Cygwin-Git
Deleting the contents of 'D:\a\Cygwin-Git\Cygwin-Git'
Initializing the repository
  C:\cygwin\bin\git.exe init D:\a\Cygwin-Git\Cygwin-Git
  hint: Using 'master' as the name for the initial branch. This default branch name
  hint: is subject to change. To configure the initial branch name to use in all
  hint: of your new repositories, which will suppress this warning, call:
  hint: 
  hint: 	git config --global init.defaultBranch <name>
  hint: 
  hint: Names commonly chosen instead of 'master' are 'main', 'trunk' and
  hint: 'development'. The just-created branch can be renamed via this command:
  hint: 
  hint: 	git branch -m <name>
  Initialized empty Git repository in /cygdrive/d/a/Cygwin-Git/Cygwin-Git/.git/
  C:\cygwin\bin\git.exe remote add origin https://github.com/me-and/Cygwin-Git
  Error: fatal: unsafe repository ('/cygdrive/d/a/Cygwin-Git/Cygwin-Git' is owned by someone else)
  To add an exception for this directory, call:
  
  	git config --global --add safe.directory /cygdrive/d/a/Cygwin-Git/Cygwin-Git
  Error: The process 'C:\cygwin\bin\git.exe' failed with exit code 1[28](https://github.com/me-and/Cygwin-Git/runs/6030136677?check_suite_focus=true#step:4:28)

[thboop]

Hey @me-and , thanks for the report!

Don't use actions/checkout, or at least maintain my own fork of actions/checkout that handles Cygwin gracefully. This'd be a shame, but I entirely understand that supporting non-default Git installations on the runners isn't what y'all signed up for.

I'd love to avoid this if we can

I don't fully understand how Cygwin works, but we copy the existing git global config from

    const gitConfigPath = path.join(
      process.env['HOME'] || os.homedir(),
      '.gitconfig'
    )

The safe.directory option supports a list, so we just add another option to that list! It shouldn't overwrite anything in that list

So i'm curious, what is breaking this part:

because when git gets called as part of actions/checkout, it doesn't look at the default global .gitconfig at all.

Do we need to find the path to the default global config from git itself, rather than trying to process.env['HOME']? Do you have a link to a run where you ran into this in a public repo? If not, I can try to spin up my own test

[me-and]

There's a run at https://github.com/me-and/Cygwin-Git/runs/6029980215 that shows this behaviour, although I had the impression that you wouldn't be able to view the logs even though it's a public repository. I can spin up a much simpler test case if that'd be useful, too.

IIRC, Windows systems typically don't have a HOME environment variable set, so I think the current process looks something like this:

• I call Cygwin Git with git config --global safe.directory .... At this point there is no HOME environment variable, so Cygwin uses its default, which is effectively something like C:\cygwin\home\username.
• The code you've quoted doesn't use process.env['HOME'], because that's not set, so it instead falls back to os.homedir(), which will be something like C:\Users\username. So it's looking at a different .gitconfig file to the one I've created.
• When Cygwin Git loads as part of this action, at that point there is a HOME environment variable, and Cygwin will respect that if it's there, so it also doesn't see the config file I specified in the first step.

(I've not actually tested the above, but it seems to make sense to me at least, and does explain the symptoms.)

This has pointed me at a couple of other possible workarounds, though:

• I can set safe.directory in the system Git config file, which I'd half-forgotten existed until now. That'll be at something like C:\cygwin\etc\gitconfig (which Cygwin applications would see as /etc/gitconfig), and would be read regardless of what HOME is (or isn't) set to.
• I can export HOME=C:\cygwin\home\username into the runner environment before calling the action, at which point the action should pick up the correct config file to use and any config I've already set in it.

I'm not sure that getting Git to give you the correct path to the config file to use will actually help here, unfortunately, at least without a lot more care: if you ask Cygwin Git where it thinks the global config file lives, it'll output something like /home/user/.gitconfig, which will be correct from the perspective of anything running within a Cygwin environment, but isn't meaningful to anything that's expecting to handle Windows paths. There are solutions to that – Cygwin provides a cygpath utility precisely for converting between Windows and Cygwin paths – but I don't think this action should be doing things that relies on handling that level of environment-specific detail.

That said, rather than getting Git to tell you the path to the config file, you could just get it to dump out the contents of the config file, which should be much less fragile. It's a bit hacky, but you could use git -c core.editor=cat config --global -e to drop the current global user config file onto stdout, then use that as the basis for the new temporary config file.

[JasonGross]

• Set safe.directory to * rather than a specific path in this action. I think that's safe – certainly I can't immediately come up with something that would break, given the config would only exist for the duration of the action run – but it might not be in the spirit of that configuration option.

This is a useful trick, thanks! (Note that whatever solution you all go for, it should handle submodules gracefully; see also #766 (comment))

[ethomson]

Set safe.directory to * rather than a specific path in this action. I think that's safe...

Unfortunately, there's a window of git versions that have the new safe.directory handling but don't understand *. 😢

Maybe both safe.directory to the specified path and safe.directory=*. 🤔 That way, users who have a new enough version of git for it to support safe.directory=* are covered, and users who have the security release that understands safe.directory (but not the wildcard) still get the current behavior.

[me-and]

Maybe both safe.directory to the specified path and safe.directory=*. 🤔 That way, users who have a new enough version of git for it to support safe.directory=* are covered, and users who have the security release that understands safe.directory (but not the wildcard) still get the current behavior.

That definitely works for anything using current Cygwin releases, doesn't sound like it'd break anything for anyone else, and while I can contrive Cygwin-based edge cases where it wouldn't work, they're very contrived – either someone self-building one of the Git releases that has safe.directory but not safe.directory=*, or going out of their way to install such a release, then adding that build to the PATH and running this action.

There's a purist part of me that doesn't like the idea of having redundant configuration lines for Git versions that support both options, and therefore either wants to test what option should be used every time or to make it something that users of this action could toggle with a with: option. But I don't think that's actually sensible, and just configuring both options regardless seems much more pragmatic.

[TingluoHuang]

What about we change actions/checkout to not set safe.directory if your global .gitconfig already have it?

[me-and]

@TingluoHuang I think that depends on how you check. Running git config --global safe.directory and checking if the output is non-empty would definitely work. Checking the contents of the .gitconfig file won't help for the same reasons the current solution doesn't work: the Windows runners don't have the HOME environment variable defined, meaning actions/checkout looks for the global .gitconfig in a different place to where Cygwin Git expects it to be.

For now, per cygporter/git@08056b4, I'm working around this by just defining HOME explicitly, and that seems to have got everything working.

[TingluoHuang]

@me-and I am planning to merge #770 which allows you to turn off the safe.directory setting we write.