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

Set PATH environment variable in make command #91

Merged
merged 2 commits into from Oct 19, 2022

Conversation

jawn-smith
Copy link
Contributor

When testing building of raspi images I noticed the make command failing to find binaries in /usr/bin. This is because now that we're setting the ARCH and SERIES environment vars PATH was being dropped.

I propose just setting PATH and ignoring the rest of the env vars in the shell that is invoking the command, but I'm open to changing if we think that is more appropriate.

@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Merging #91 (10c3ff5) into feature/classic-image-redesign (1cc195c) will not change coverage.
The diff coverage is 100.00%.

@@                       Coverage Diff                        @@
##           feature/classic-image-redesign       #91   +/-   ##
================================================================
  Coverage                          100.00%   100.00%           
================================================================
  Files                                   9         9           
  Lines                                1694      1695    +1     
================================================================
+ Hits                                 1694      1695    +1     
Impacted Files Coverage Δ
internal/statemachine/classic_states.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jawn-smith
Copy link
Contributor Author

@waveform80 Prefers using the whole shell env plus ARCH and SERIES, so I did that.

Copy link
Collaborator

@sil2100 sil2100 left a comment

Choose a reason for hiding this comment

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

I guess that's fine, but I'm of course thinking from an attack-vector POV a bit. I suppose building the gadget with make (and sudo) is already dangerous enough, so just adding the env should not make much difference here.

@jawn-smith
Copy link
Contributor Author

I guess that's fine, but I'm of course thinking from an attack-vector POV a bit. I suppose building the gadget with make (and sudo) is already dangerous enough, so just adding the env should not make much difference here.

Security POV was my initial reasoning for only including PATH. However, @waveform80 changed my mind by bringing up that everything is already being run with root privileges.

@waveform80
Copy link
Member

Indeed -- it's running as root so if your attacker can affect root's environment, your goose is cooked anyway :)

@jawn-smith jawn-smith merged commit c4d1265 into feature/classic-image-redesign Oct 19, 2022
@upils upils deleted the make-environment branch September 18, 2023 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants