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

RFC: Removed the default set PATH for Windows environments. #4895

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danielGithinji
Copy link
Contributor

@danielGithinji danielGithinji commented May 2, 2024

This change is related to this PR: Don't set a default PATH for Windows #3158.

Description:

This change seeks to remove the defaulted Windows image path found in util\system\path.go.

When the PATH is defaulted to a specific value, application files for applications like PowerShell and any other installed application cannot be found/referenced. The reason for this is that Windows saves its environment variables in the registry.

Let's look at some examples to drive this point home.

Example 1: Get access to powershell

I used buildkit to create two images with following dockerfile contents:

FROM mcr.microsoft.com/windows/servercore:ltsc2022

RUN echo %PATH%

CMD ["powershell", "-Command", "Write-Host", "$env:PATH"]

  1. First Image
  2. The PATH in util\system\path.go is defaulted to "c:\Windows\System32;c:\Windows"

    When you run this image you get the following result:

    PS C:\Users\dgithinji> ctr run docker.io/gmanguru/fix-windows-path:withDefaultPath withDefaultPath
    ctr: hcs::System::CreateProcess: powershell -Command Write-Host $env:PATH withDefaultPath: The system cannot find the file specified.: unknown
    
  3. Second Image
  4. > The PATH in _util\system\path.go_ is defaulted to an empty string

    Running the image gives the following result:

    PS C:\Users\dgithinji> ctr run docker.io/gmanguru/fix-windows-path:NoDefaultPath NoDefaultPath
    C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps
    

In mcr.microsoft.com/windows/servercore:ltsc2022, PowerShell is available in "C:\Windows\System32\WindowsPowerShell\v1.0". By setting the PATH to a default it makes sure that powershell and any other default application isn't available in the env PATH variable.

Example 2: Installing an application or binary

I created an image that has an installed Go binary using the following dockerfile:

Dockerfile:
FROM mcr.microsoft.com/windows/servercore:ltsc2022

RUN echo %PATH%

SHELL ["powershell", "-Command", "$ErrorActionPreference = 'Stop'; $ProgressPreference = 'Continue'; $verbosePreference='Continue';"]

RUN REG QUERY \"HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\Environment\" /V PATH

# Set the URL for the Go binary download
ENV GO_URL=https://go.dev/dl/go1.23.0.windows-amd64.msi

# Create a directory for Go installation
RUN mkdir C:\go

COPY go1.23.0.windows-amd64.msi \\go\\go.msi

# Download the Go binary and extract it to C:\go
RUN powershell -Command start-process msiexec.exe -Wait -Argumentlist '/i','C:\go\go.msi','/quiet'; REG QUERY \"HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\Environment\" /V PATH

RUN echo $env:PATH; REG QUERY \"HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\Environment\" /V PATH

# Verify the Go installation
RUN go version

  1. First Image:
  2. The PATH in util\system\path.go is defaulted to "c:\Windows\System32;c:\Windows"

    Image build results:
      buildctl build `
    >> --frontend=dockerfile.v0 `
    >> --local context=. \ `
    >> --local dockerfile=. `
    >> --output type=image,name=servercore-go,push=false `
    >> --progress plain `
    >> --no-cache
    
    #1 [internal] load build definition from Dockerfile
    #1 transferring dockerfile: 1.13kB 0.0s done
    #1 DONE 0.0s
    
    #2 [internal] load metadata for mcr.microsoft.com/windows/servercore:ltsc2022
    #2 DONE 1.0s
    
    #3 [internal] load .dockerignore
    #3 transferring context: 2B done
    #3 DONE 0.0s
    
    #4 [1/8] FROM mcr.microsoft.com/windows/servercore:ltsc2022@sha256:a57f79008ec0110877d5907787095c413fae1c27cccf1473bc9646fd7325febe
    #4 resolve mcr.microsoft.com/windows/servercore:ltsc2022@sha256:a57f79008ec0110877d5907787095c413fae1c27cccf1473bc9646fd7325febe 0.0s done
    #4 CACHED
    
    #5 [internal] load build context
    #5 transferring context: 50B done
    #5 DONE 0.0s
    
    #6 [2/8] RUN echo %PATH%
    #6 1.444 C:\Windows\System32\WindowsPowerShell\v1.0\;
    #6 DONE 2.1s
    
    #7 [3/8] RUN REG QUERY "HKLMSYSTEMCurrentControlSetControlSession ManagerEnvironment" /V PATH
    #7 8.359 REG : The term 'REG' is not recognized as the name of a cmdlet, function,
    #7 8.359 script file, or operable program. Check the spelling of the name, or if a path
    #7 8.359 was included, verify that the path is correct and try again.
    #7 8.359 + ... ssPreference = 'Continue'; $verbosePreference='Continue'; REG QUERY " ...
    #7 8.359 At line:1 char:99
    #7 8.359 +                                                               ~~~
    #7 8.360    rrorRecordException
    #7 8.360     + CategoryInfo          : ObjectNotFound: (REG:String) [], ParentContainsE
    #7 8.360     + FullyQualifiedErrorId : CommandNotFoundException
    #7 8.360
    #7 ERROR: process "powershell -Command $ErrorActionPreference = 'Stop'; $ProgressPreference = 'Continue'; $verbosePreference='Continue'; REG QUERY \\\"HKLM\\SYSTEM\\CurrentControlSet\\Control\\Session Manager\\Environment\\\" /V PATH" did not complete successfully: exit code: 1
    ------
    > [3/8] RUN REG QUERY "HKLMSYSTEMCurrentControlSetControlSession ManagerEnvironment" /V PATH:
    8.359 REG : The term 'REG' is not recognized as the name of a cmdlet, function,
    8.359 script file, or operable program. Check the spelling of the name, or if a path
    8.359 was included, verify that the path is correct and try again.
    8.359 + ... ssPreference = 'Continue'; $verbosePreference='Continue'; REG QUERY " ...
    8.359 At line:1 char:99
    8.359 +                                                               ~~~
    8.360    rrorRecordException
    8.360     + CategoryInfo          : ObjectNotFound: (REG:String) [], ParentContainsE
    8.360     + FullyQualifiedErrorId : CommandNotFoundException
    8.360
    ------
    Dockerfile:9
    --------------------
     7 |     SHELL ["powershell", "-Command", "$ErrorActionPreference = 'Stop'; $ProgressPreference = 'Continue'; $verbosePreference='Continue';"]
     8 |
     9 | >>> RUN REG QUERY \"HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\Environment\" /V PATH
    10 |
    11 |
    --------------------
    error: failed to solve: process "powershell -Command $ErrorActionPreference = 'Stop'; $ProgressPreference = 'Continue'; $verbosePreference='Continue'; REG QUERY \\\"HKLM\\SYSTEM\\CurrentControlSet\\Control\\Session Manager\\Environment\\\" /V PATH" did not complete successfully: exit code: 1
    
  3. Second Image
  4. The PATH in util\system\path.go is defaulted to an empty string

    Image build results:
       buildctl build `
    >> --frontend=dockerfile.v0 `
    >> --local context=. \ `
    >> --local dockerfile=. `
    >> --output type=image,name=servercore-go,push=false `
    >> --progress plain `
    >> --no-cache
    #1 [internal] load build definition from Dockerfile
    #1 transferring dockerfile: 1.08kB done
    #1 DONE 0.1s
    
    #2 [internal] load metadata for mcr.microsoft.com/windows/servercore:ltsc2022
    #2 DONE 0.9s
    
    #3 [internal] load .dockerignore
    #3 transferring context: 2B done
    #3 DONE 0.0s
    
    #4 [1/8] FROM mcr.microsoft.com/windows/servercore:ltsc2022@sha256:b7b2e5b4c2414400c4eef13db747376e0f10ef8e15b8d0587ef5b953ad4e6d43
    #4 resolve mcr.microsoft.com/windows/servercore:ltsc2022@sha256:b7b2e5b4c2414400c4eef13db747376e0f10ef8e15b8d0587ef5b953ad4e6d43 0.0s done
    #4 CACHED
    
    #5 [internal] load build context
    #5 transferring context: 50B done
    #5 DONE 0.0s
    
    #6 [2/8] RUN echo %PATH%
    #6 1.106 C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps
    #6 DONE 2.0s
    
    #7 [3/8] RUN REG QUERY "HKLMSYSTEMCurrentControlSetControlSession ManagerEnvironment" /V PATH
    #7 3.874
    #7 3.874 HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment
    #7 3.874     PATH    REG_EXPAND_SZ    %SystemRoot%\system32;%SystemRoot%;%SystemRoot%\System32\Wbem;%SYSTEMROOT%\System32\WindowsPowerShell\v1.0\;%SYSTEMROOT%\System32\OpenSSH\
    #7 3.875
    #7 DONE 4.8s
    
    #8 [4/8] RUN mkdir C:go
    #8 4.755
    #8 4.759
    #8 4.766     Directory: C:\
    #8 4.766
    #8 4.766
    #8 4.772 Mode                 LastWriteTime         Length Name
    #8 4.773 ----                 -------------         ------ ----
    #8 4.775 d-----         9/26/2024   1:08 PM                go
    #8 4.779
    #8 4.779
    #8 DONE 5.6s
    
    #9 [5/8] COPY go1.23.0.windows-amd64.msi \go\go.msi
    #9 DONE 2.0s
    
    #10 [6/8] RUN powershell -Command start-process msiexec.exe -Wait -Argumentlist '/i','C:\go\go.msi','/quiet'; REG QUERY "HKLMSYSTEMCurrentControlSetControlSession ManagerEnvironment" /V PATH
    #10 89.28
    #10 89.28 HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment
    #10 89.28     PATH    REG_EXPAND_SZ    %SystemRoot%\system32;%SystemRoot%;%SystemRoot%\System32\Wbem;%SYSTEMROOT%\System32\WindowsPowerShell\v1.0\;%SYSTEMROOT%\System32\OpenSSH\;C:\Program Files\Go\bin
    #10 89.28
    #10 DONE 90.6s
    
    #11 [7/8] RUN echo $env:PATH; REG QUERY "HKLMSYSTEMCurrentControlSetControlSession ManagerEnvironment" /V PATH
    #11 66.25 C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Program Files\Go\bin;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps;C:\Users\ContainerAdministrator\go\bin
    #11 66.27
    #11 66.27 HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment
    #11 66.27     PATH    REG_EXPAND_SZ    %SystemRoot%\system32;%SystemRoot%;%SystemRoot%\System32\Wbem;%SYSTEMROOT%\System32\WindowsPowerShell\v1.0\;%SYSTEMROOT%\System32\OpenSSH\;C:\Program Files\Go\bin
    #11 66.27
    #11 DONE 67.3s
    
    #12 [8/8] RUN go version
    #12 5.250 go version go1.23.0 windows/amd64
    #12 DONE 6.2s
    
    #13 exporting to image
    #13 exporting layers
    #13 exporting layers 45.9s done
    #13 exporting manifest sha256:e15cb18d680b51345c88959adb9ea04b9a2a4b1bfbb3184360935c9b6764c592 done
    #13 exporting config sha256:5cabdc7ee9b1d199f9fdb90cc5299143ade03d5a33838ffe5e6716b3d3ec34c5 0.0s done
    #13 naming to servercore-go done
    #13 DONE 45.9s
    

From Example 2 above, its clear to see how installing binaries/applications where the PATH has a default value ensures that the application can't be accessed from the env path variable, unless its referenced from the specific location that its in installed in.
When the PATH isn't set the installed application path is set in both the registry item as well as path that is available when running the image.

Example 3:

The following examples make use of setx /m command that is specific to Windows to update the path.
Here's the dockerfile that I used:

FROM mcr.microsoft.com/windows/servercore:ltsc2022

RUN echo %PATH%

CMD ["powershell", "-Command", "Write-Host", "$env:PATH"]

  1. First Image
  2. The PATH in util\system\path.go is defaulted to "c:\Windows\System32;c:\Windows"

    Image build results:
    buildctl build `
    >> --frontend=dockerfile.v0 `
    >> --local context=. \ `
    >> --local dockerfile=. `
    >> --output type=image,name=servercore-go,push=false `
    >> --progress plain `
    >> --no-cache
    #1 [internal] load build definition from Dockerfile
    #1 transferring dockerfile: 142B 0.0s done
    #1 DONE 0.0s
    
    #2 [internal] load metadata for mcr.microsoft.com/windows/servercore:ltsc2022
    #2 DONE 0.8s
    
    #3 [internal] load .dockerignore
    #3 transferring context: 2B done
    #3 DONE 0.0s
    
    #4 [1/3] FROM mcr.microsoft.com/windows/servercore:ltsc2022@sha256:b7b2e5b4c2414400c4eef13db747376e0f10ef8e15b8d0587ef5b953ad4e6d43
    #4 resolve mcr.microsoft.com/windows/servercore:ltsc2022@sha256:b7b2e5b4c2414400c4eef13db747376e0f10ef8e15b8d0587ef5b953ad4e6d43 0.0s done
    #4 CACHED
    
    #5 [2/3] RUN setx /M PATH "C:\my\path;%PATH%"
    #5 1.140
    #5 1.140 SUCCESS: Specified value was saved.
    #5 DONE 1.9s
    
    #6 [3/3] RUN echo %PATH%
    #6 2.644 c:\Windows\System32;c:\Windows
    #6 DONE 3.0s
    
    #7 exporting to image
    #7 exporting layers
    #7 exporting layers 2.3s done
    #7 exporting manifest sha256:630a9a0aefb5ad58e5d4bc265b1c0e64b5e01031a1f046361629eb6ed7170f42 done
    #7 exporting config sha256:00aeb83d90e3353b8b2769c6e4a9c47e8420cd3c07ffe6cf286bca93cd8c552f done
    #7 naming to servercore-go done
    #7 DONE 2.3s
    

    In this case the setx command does not update the path env variable

  3. Second Image
  4. When the PATH is defaulted to an empty, setx command is able to update the PATH env variable as follows:
    Image build results:
    buildctl build `
    >> --frontend=dockerfile.v0 `
    >> --local context=. \ `
    >> --local dockerfile=. `
    >> --output type=image,name=servercore-go,push=false `
    >> --progress plain `
    >> --no-cache
    #1 [internal] load build definition from Dockerfile
    #1 transferring dockerfile: 142B 0.0s done
    #1 DONE 0.0s
    
    #2 [internal] load metadata for mcr.microsoft.com/windows/servercore:ltsc2022
    #2 DONE 0.3s
    
    #3 [internal] load .dockerignore
    #3 transferring context: 2B done
    #3 DONE 0.0s
    
    #4 [1/3] FROM mcr.microsoft.com/windows/servercore:ltsc2022@sha256:b7b2e5b4c2414400c4eef13db747376e0f10ef8e15b8d0587ef5b953ad4e6d43
    #4 resolve mcr.microsoft.com/windows/servercore:ltsc2022@sha256:b7b2e5b4c2414400c4eef13db747376e0f10ef8e15b8d0587ef5b953ad4e6d43 0.0s done
    #4 CACHED
    
    #5 [2/3] RUN setx /M PATH "C:\my\path;%PATH%"
    #5 1.110
    #5 1.110 SUCCESS: Specified value was saved.
    #5 DONE 1.9s
    
    #6 [3/3] RUN echo %PATH%
    #6 2.592 C:\my\path;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\Users\ContainerAdministrator\AppData\Local\Microsoft\WindowsApps;
    #6 DONE 3.0s
    
    #7 exporting to image
    #7 exporting layers
    #7 exporting layers 2.2s done
    #7 exporting manifest sha256:08ff4cbbaa13bc91694cd8d67ef3fb12c2e5c2a4624687a559ef23d74e570e57 done
    #7 exporting config sha256:8d0ff2bb8910b97677a445b7381417ae943f89f75041466b761a851c3b3aec73 done
    #7 naming to servercore-go done
    #7 DONE 2.2s
    

When the path isn't defaulted, the setx command works as it should giving it backwards compatibility with how classic docker works.

Conclusion:

  • There are several versions of windows containers each with its own use-case. For example servercore(that used in the examples above) is used because it contains powershell whereas images with the .net libraries are used for that particular function. Each of these images have different installed native applications and libraries.

  • When the PATH is defaulted accessibility to image applications is lost and a lot of manual steps such as using the full path for each application has to be done in order to use them.

  • Therefore removing the defaulted Windows image path:

    1. Will make default/installed applications be accessible because they'll be included in the PATH env variable i.e powershell and any other application will be available directly from the shell.
    2. Will allow backwards compatibility with how classic docker works specifically for Windows native commands like setx.

@danielGithinji danielGithinji changed the title Removed the default set PATH for Windows environments to allow Window… Removed the default set PATH for Windows environments. May 2, 2024
@danielGithinji danielGithinji marked this pull request as draft May 2, 2024 08:07
@danielGithinji danielGithinji changed the title Removed the default set PATH for Windows environments. RFC: Removed the default set PATH for Windows environments. May 2, 2024
@danielGithinji danielGithinji force-pushed the fix-overriding-default-windows-path branch from 3e47de5 to 7b1de15 Compare May 2, 2024 11:48
@danielGithinji danielGithinji force-pushed the fix-overriding-default-windows-path branch from 2e4fe59 to e2a0a84 Compare August 14, 2024 21:35
Daniel Githinji added 2 commits September 27, 2024 20:02
…ecks whether the path in a Windows container image can be set or not.

Signed-off-by: Daniel Githinji <[email protected]>
…s images to set the PATH while running

Signed-off-by: Daniel Githinji <[email protected]>
@danielGithinji danielGithinji force-pushed the fix-overriding-default-windows-path branch from 1c4cb16 to ddb7a7c Compare September 27, 2024 17:03
@thaJeztah
Copy link
Member

@profnandaa
Copy link
Collaborator

Also see ;

Hi @thaJeztah -- we've been trying to get some context from the previous PR but was getting lost in the discussions. If you don't mind, able to help with a summary? Esp.

  • what do you think we need to have backward compatibility with the current/classic docker builder?
  • how do we solve first for setx and may be look into ENV as a follow up?

Thanks for TAL.

allTests = append(allTests, windowsTests...)
}

func testSetPath(t *testing.T, sb integration.Sandbox) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW this needs to only run on Windows.

Copy link
Collaborator

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

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

I was therefore proposing this #5445

// ';' character .
const DefaultPathEnvWindows = "c:\\Windows\\System32;c:\\Windows"
// DefaultPathEnvWindows is empty to allow windows to set the PATH when it runs
const DefaultPathEnvWindows = ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: so this ensures backward compatibility with classic docker build, but on the downside, you can't do something like this:

ENV PATH "$PATH;C:\\add\\my\\path"

You end up with PATH=;C:\\add\\my\\path, and the one coming from the registry is now ignored. We therefore end up with some inconsistencies.

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.

4 participants