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

fix(issue 295): ensure inputs don't leak to child processes #296

Merged
merged 7 commits into from
May 4, 2023

Conversation

AlanSl
Copy link
Contributor

@AlanSl AlanSl commented May 3, 2023

closes #295

I split this out from the provenance work because it's a separate issue, posting this as an example implementation if we do think it's something worth doing. Not sure if it should have tests or not or if it's really possible to test?

Copy link
Contributor

@simoneb simoneb left a comment

Choose a reason for hiding this comment

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

Is it feasible/valuable to have a test for this?

@AlanSl
Copy link
Contributor Author

AlanSl commented May 3, 2023

Not sure. Ultimately it depends on an internal behaviour of GitHub Actions that ends in a child process. so I don't think we can test that? At least I don't think we can do any kind of full integration test here.

Maybe we can and should have a basic unit test that stubs @actions/exec, sets some env vars similar to how I did in the provenance tests, some of which are INPUT_ and some aren't, calls execWithOutput, and ensures that the exec is called with the non-input properties present and the input properties missing?

@simoneb
Copy link
Contributor

simoneb commented May 3, 2023

yes please, let's have a unit test for this behavior

@AlanSl AlanSl requested a review from simoneb May 3, 2023 16:37
@simoneb
Copy link
Contributor

simoneb commented May 3, 2023

I'm sure you noticed but CI is red @AlanSl

@simoneb simoneb merged commit e25b87f into nearform-actions:main May 4, 2023
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.

Prevent leaking secrets by stripping INPUT_* env vars when running @actions/exec
2 participants