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

Filter sensitive values from being logged in ProcessProxy #1279

Merged
merged 27 commits into from
Mar 14, 2023

Conversation

starskyreverie
Copy link
Contributor

@starskyreverie starskyreverie commented Mar 14, 2023

Allows users to set kwarg sensitive_env for ProcessProxy, an array of phrases indicating secrets that users may not want logged.

Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

Hi @pq43 - thank you for the pull request. This seems like a decent idea although I think it needs to be configurable by an operator

@kevin-bates kevin-bates added enhancement security gateway-provisioners Has application for gateway-provisioners (EG 4.0) labels Mar 14, 2023
@starskyreverie starskyreverie requested review from lresende and kevin-bates and removed request for lresende March 14, 2023 16:15
@kevin-bates
Copy link
Member

I guess I meant for these to be manifested as configurable traits (that can be configured via a configuration file), but initially rolling with envs is probably fine so let's skip the trait aspect of this.

Please add documentation for each of the new envs to this page: https://github.com/jupyter-server/enterprise_gateway/blob/main/docs/source/operators/config-add-env.md

@starskyreverie
Copy link
Contributor Author

Oh okay, sounds good! To do what you said via traitlets, should I define EG_SENSITIVE_ENV_KEYS in mixins.py or is there a better place?

@kevin-bates
Copy link
Member

To do what you said via traitlets, should I define EG_SENSITIVE_ENV_KEYS in mixins.py or is there a better place?

Yes, but I'm thinking it might be best to just keep these as envs for now. These will need to reside in Gateway Provisioners when EG moves to those and we could decide to use traits at that time, but introducing traits now will only create migration headaches.

Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

Looks good @pq43 - thank you.

@lresende lresende merged commit a09cfb7 into jupyter-server:main Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement gateway-provisioners Has application for gateway-provisioners (EG 4.0) security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants