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

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

Merged
merged 1 commit into from
May 27, 2023

Conversation

thaJeztah
Copy link
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
Member Author

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

@codecov-commenter
Copy link

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 <github@gone.nl>
@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