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

Read action's secrets from env vars and avoid logging them #2333

Merged
merged 12 commits into from
Aug 11, 2021

Conversation

itaiad200
Copy link
Contributor

@itaiad200 itaiad200 commented Aug 8, 2021

closes #2282

This implementation assumes that secrets are to be used only in specific locations and are not available in all places. For example, query-params & headers are good place for secrets, but the url itself isn't.

While this might limit the users a bit when trying to pass variables to the execution itself, it will allow us to log all other parameters of the actions, without worrying about leakage of secrets. Env var references in other places in the action yaml file are simply ignored.

@itaiad200 itaiad200 added the area/hooks improvements or additions to the hooks subsystem label Aug 8, 2021
@@ -375,6 +375,7 @@ jobs:
TAG: ${{ needs.deploy-image.outputs.tag }}
# Setting Account_ID as a secret as a way to avoid specifying it here
REPO: ${{ secrets.AWS_ACCOUNT_ID }}.dkr.ecr.us-east-1.amazonaws.com
ACTIONS_VAR: "this_is_actions_var"
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, inject this env var from the test code and not from here - otherwise when running locally, one has to make sure the var is set in order for the test to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lakeFS is running separately from the test code. I can't control it from the test code, I can set it always in lakeFS but that seems like too much. WDYT?

pkg/actions/secure_string.go Outdated Show resolved Hide resolved
pkg/actions/secure_string.go Outdated Show resolved Hide resolved
pkg/actions/testdata/action_secrets.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@johnnyaug johnnyaug left a comment

Choose a reason for hiding this comment

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

Looking good.
Missing: documentation, CHANGELOG entry.

pkg/actions/secure_string.go Outdated Show resolved Hide resolved
pkg/actions/secure_string.go Outdated Show resolved Hide resolved
pkg/actions/secure_string.go Outdated Show resolved Hide resolved
@itaiad200 itaiad200 merged commit fb6f4ed into master Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hooks improvements or additions to the hooks subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better action files secret handling
2 participants