Powershell: Translating PSDrive-based path arguments for external (native) executables is overzealous (PSNativePSPathResolution)

[mklement0]

PR #12386 introduced _experimental_ feature PSNativePSPathResolution, which as of (at least) PowerShell Core 7.1.0-preview.7 has become a _default_ feature.

• For problems with this feature in general, see #13644

Its purpose is to translate _PowerShell-drive-based_ paths to _native filesystem_ paths, because external executables only understand the latter.

This translation is currently overzealous in that it nonsensically translates an argument that _starts with :_ (which clearly isn't a _drive-based path_).

@DHowett has pinpointed the problematic behavior to: https://github.com/PowerShell/PowerShell/blob/3effa204103460c996a8612aa70718fdf924047d/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs#L388-L406

Steps to reproduce

On Unix:

/bin/echo :foo | Should -Be ':foo'

On Windows:

cmd /c echo :foo | Should -Be ':foo'

Expected behavior

The test should succeed.

Actual behavior

The test fails, because the the current location's path is unexpectedly prepended to :

Expected strings to be the same, but they were different. Expected length: 4 Actual length:   44 Strings differ at index 0.
Expected: ':foo' But was:  '/Users/jdoe...'

Environment data

PowerShell Core 7.1.0-preview.7

[iSazonov]

The bug is not in PSNativePSPathResolution. It is in globber.GetProviderPath(). Try dir : for demo.

[mklement0]

You're right: the bug also affects The behavior also applies to arguments to _cmdlets_ that expect _item paths_, such as Get-ChildItem (dir).

And it also surfaces if there's no globbing involved whatsoever: Get-ChildItem -LiteralPath :.

As such, it goes back to Windows PowerShell.

[Update: : is simply interpreted as the start of a relative path.]

However, with respect to _external executables_ it only surfaces since PSNativePSPathResolution was made a regular feature.

[iSazonov]

It is not clear how should we react on ':name' or 'name:name' in both cases.

[iSazonov]

In IsAbsolutePath() I see the comment:

                // If the : is the first character in the path then we
                // must assume that it is part of the path, and not
                // delimiting the drive name.

So for dir : scenario it is by design and for first scenario we could do the same if first char is ':' but it it is still not clear what we should do for name:name in both scenarios.

[iSazonov]

Current behavior for path like name1:name2 - throw if drive name1 is not found. Should we change this and consider name1:name2 as file/directory name (if drive name1 is not found) because we allow :name?

[vexx32]

I think we're missing the point here quite a bit tbh.

The problem that was originally raised on #13638 is that because we're looking for paths on all input parameters, we're breaking some use cases where an argument string is literally just thing:thing or even just :thing because PS is treating that as a relative path and it's being resolved to a full path.

@DHowett's original example was:

& git log ":(top,exclude).git/" ":(top,exclude)**/*.rec"

That's not meant to be a _path_ in the same way that PowerShell tends to think of paths. Some of these need to passed along _without_ being resolved at all, not as a file name/directory name or anything. With PSNativePSPathResolution a standard feature, if PS thinks it's a relative path it's _resolving_ it to what it thinks the full path "should" be. Which means an argument like this will never be passed to the executable verbatim. Which is going to be endlessly problematic.

If we get name1:name2 and name1 isn't a recognised physical drive or PSDrive, IMO it should be passed along verbatim.

[iSazonov]

Which is going to be endlessly problematic.

It seems only reliable way to implement PSNativePSPathResolution it is to know that native application expects a path.

[mklement0]

If we get name1:name2 and name1 isn't a recognised physical drive or PSDrive, IMO it should be passed along verbatim.

Indeed, @vexx32, and that is actually how it _mostly_ already works (except that action is only required for _PSDrives_, not native filesystem drives), with an argument-initial : constituting the only exception that amounts to a _bug_, as described in the OP.

So the bottom line is: An argument that _starts with :_ is by definition _not_ a path based on a PS drive and therefore no attempt should be made to translate it to a native path.

Note, however, that that doesn't necessarily mean _verbatim_, because on Unix _globbing_ may still have to be performed on an (unquoted) :-prefixed argument, given that there you may have, say, a subdir named :foo containing .txt files, which should be glob-expanded by something like /bin/echo :foo/*.txt (to pass individual arguments :foo/bar.txt, ...)

---

I got confused by the behavior of Get-ChildItem : (and other provider cmdlets), but that is really a separate issue:

A filesystem-path-argument-initial : is interpreted as a _relative path_, but on Windows (unlike on Unix), file/directory _names_ mustn't contain :, so there is no point in trying to interpret as such.

Doing so is probably not a big deal in practice, (cmd.exe similarly yields a File Not Found), but the current error message, which claims _nonexistence_ (Cannot find path '...' because it does not exist) with the _full path_ based on the current location is a bit confusing.

[vexx32]

Oh, : is a valid item name on Unix? Didn't know that. I guess I assumed it wasn't considered valid because that's what the path separator usually is on Unix systems. 🤔

[mklement0]

It seems only reliable way to implement PSNativePSPathResolution it is to know that native application expect a path.

@iSazonov, unfortunately, that is impossible, and the current feature is _guesswork_, for sure, but I suspect that it will work seamlessly in the vast majority of cases.

That said, it's important:

• To document the magic in detail.

• To provide a way to _bypass_ the magic on demand.

  • Currently, using _single_-quoting ('...') rather than double-quoting ("...") provides this bypass, but I think that is conceptually problematic and obscure and only works with _literal_ arguments to boot.

Oh, : is a valid item name on Unix? Didn't know that. I guess I assumed it wasn't considered valid because that's what the path separator usually is on Unix systems

@vexx32, yes, that's surprising, but note that, similarly, ; is a valid filename character on Windows.

[DHowett]

Incidentally, single quoting won't work for the use case that led me to discover the issue.

My original code was closer to...

& git log (Get-ExcludedLogPatterns)

...which I do not believe I can single-quote (or use --% on!)

[iSazonov]

unfortunately, that is impossible, and the current feature is guesswork, for sure,

I will reference this from #13428 native cmdlet class :-)

unfortunately, that is impossible, and the current feature is guesswork, for sure, but I suspect that it will work seamlessly in the vast majority of cases.

In the case we need an way to temporary disable the feature. It can fall us in bad history like --% and quotes.

[mklement0]

@DHowett, with additional work - which invariably involves creating an auxiliary environment variable that pollutes the child process' environment - you _can_ use --% to work around the problem, but it is obviously cumbersome :

PS> $env:_aux_arg = ':foo'; cmd --% /c echo %_aux_arg%
:foo

---

@iSazonov, implementing wrappers around all external CLIs that accept file-system paths is obviously impossible.

In the case we need a way to temporary disable the feature. It can fall us in bad history like --% and quotes.

I agree, and, as stated, the current mechanism - single-quoting - is both obscure and inadequate, due to only working with _literals_.

I'm not sure there's a good _in-call_ solution, and that makes me wonder whether this feature - as helpful as it may be in many cases - is worth implementing at all.

[iSazonov]

implementing wrappers around all external CLIs that accept file-system paths is obviously impossible.

Yes, but the intention is to have API so that users can easily implement helpers for important/popular native application. For the case with git we could explicitly define parameters and specify which parameters the paths accept so that the engine can convert them correctly.

[mklement0]

@iSazonov, I think that these wrappers are the wrong way to go, as previously argued in #13428, but we should continue this discussion there.

As for the issue at hand:

• Even in the context of the current PSNativePSPathResolution implementation, the behavior with an :-initial argument is clearly a _bug_.

• I've created #13644, where we can continue the discussion about the general problems with PSNativePSPathResolution.

[DHowett]

I do not believe that Issue-Question is the appropriate label for this issue. This is a significant regression in native tool use from one preview release to another, and we are at risk of shipping with it.

[vexx32]

/cc @SteveL-MSFT

[DHowett]

@SteveL-MSFT, @joeyaiello - since we just hit RC1, here's a gentle ping on potentially shipping catastrophic and unavoidable behavior for invoking native executables. :smile:

[SteveL-MSFT]

For 7.1, I'm going to revert the change to make this non-experimental

[TravisEz13]

We marked #13734 for back port to the 7.1 release branch

[iSazonov]

@TravisEz13 You say about 7.1 label here but a label in the PR is 7.0 - typo?