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

Drop usage of command.exec in codebase where possible #4908

Open
nitrocode opened this issue Sep 8, 2024 · 3 comments
Open

Drop usage of command.exec in codebase where possible #4908

nitrocode opened this issue Sep 8, 2024 · 3 comments
Labels
bug Something isn't working security

Comments

@nitrocode
Copy link
Member

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Overview of the Issue

The atlantis project has always used the command.exec golang functions. These are error prone and have lead to security issues in the past. It would be best to replace these with native golang libraries.

There are currently 19 files that use it

https://github.com/search?q=repo%3Arunatlantis%2Fatlantis+exec.command+language%3AGo&type=code

Use-cases are

Reproduction Steps

N/A

Logs

N/A

Environment details

N/A

Additional Context

@nitrocode nitrocode added bug Something isn't working security labels Sep 8, 2024
@abstractionfactory
Copy link

I'd like to note that tofu-exec isn't really well maintained at this point and it relies on exec'ing anyway, so would defer to using TofuDL + exec'ing instead.

@nitrocode
Copy link
Member Author

Hi @abstractionfactory , thanks for commenting.

The benefits of using a 3rd party official library for this, even if it uses the same underlying mechanism is that we can presume, due to more usage and official ownership, that it's probably better maintained than our bespoke implementation. Our implementation is also inconsistent and could lead to mistakes resulting in security issues.

From the looks of https://github.com/opentofu/tofu-exec, it appears it's a fork of the terraform version with commits to keep it updated for opentofu. The last commit was from 4 months ago which is relatively recent. For comparison, there are libraries we depend on that haven't been updated in years. See #2795.

@nitrocode
Copy link
Member Author

I'm looking through the code and it uses a pattern where it defines each subcommand as an individual function to help insulate each passed in argument before calling the underlying exec.CommandContext() which is better practice.

https://github.com/opentofu/tofu-exec/blob/cf3d41b2300ec849ca43b5d192adef69dfb63560/tfexec/state_pull.go#L54-L58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working security
Projects
None yet
Development

No branches or pull requests

2 participants