Skip to content

remove uses of golang.org/x/sys/execabs#270

Merged
thaJeztah merged 1 commit intodocker:masterfrom
thaJeztah:remove_execabs
May 27, 2023
Merged

remove uses of golang.org/x/sys/execabs#270
thaJeztah merged 1 commit intodocker:masterfrom
thaJeztah:remove_execabs

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

the "golang.org/x/sys/execabs" package was introduced to address a security issue on Windows, and changing the default behavior of os/exec was considered a breaking change. go1.19 applied the behavior that was previously implemented in the execabs package;

from the release notes: https://go.dev/doc/go1.19#os-exec-path

Command and LookPath no longer allow results from a PATH search to be found
relative to the current directory. This removes a common source of security
problems but may also break existing programs that depend on using, say,
exec.Command("prog") to run a binary named prog (or, on Windows, prog.exe)
in the current directory. See the os/exec package documentation for information
about how best to update such programs.

On Windows, Command and LookPath now respect the NoDefaultCurrentDirectoryInExePath
environment variable, making it possible to disable the default implicit search
of “.” in PATH lookups on Windows systems.

With those changes, we no longer need to use the execabs package, and we can switch back to os/exec.

@thaJeztah thaJeztah requested a review from crazy-max May 26, 2023 00:08
@thaJeztah
Copy link
Copy Markdown
Member Author

actually; let me change go.mod to go1.19 (as 1.18 doesn't have that fix)

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 26, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (da93839) 55.55% compared to head (3d84f97) 55.55%.

❗ Current head 3d84f97 differs from pull request most recent head 37c4a6b. Consider uploading reports for the commit 37c4a6b to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #270   +/-   ##
=======================================
  Coverage   55.55%   55.55%           
=======================================
  Files           9        9           
  Lines         666      666           
=======================================
  Hits          370      370           
  Misses        253      253           
  Partials       43       43           
Impacted Files Coverage Δ
client/command.go 0.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

the "golang.org/x/sys/execabs" package was introduced to address a security
issue on Windows, and changing the default behavior of os/exec was considered
a breaking change. go1.19 applied the behavior that was previously implemented
in the execabs package;

from the release notes: https://go.dev/doc/go1.19#os-exec-path

> Command and LookPath no longer allow results from a PATH search to be found
> relative to the current directory. This removes a common source of security
> problems but may also break existing programs that depend on using, say,
> exec.Command("prog") to run a binary named prog (or, on Windows, prog.exe)
> in the current directory. See the os/exec package documentation for information
> about how best to update such programs.
>
> On Windows, Command and LookPath now respect the NoDefaultCurrentDirectoryInExePath
> environment variable, making it possible to disable the default implicit search
> of “.” in PATH lookups on Windows systems.

With those changes, we no longer need to use the execabs package, and we can
switch back to os/exec.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah merged commit 7ce5629 into docker:master May 27, 2023
@thaJeztah thaJeztah deleted the remove_execabs branch May 27, 2023 09:47
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.

4 participants