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

bugfix: sanitise log/console secrets #535

Merged

Conversation

wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

fixes #529

list of issues fixed by this PR:

  • logging connection string URIs with exposed secrets
  • command (kubectl) logging that contains kubeconfig files with certificates

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf added enhancement New feature or request go Pull requests that update Go code security Pertains project's security posture in some way labels Jan 24, 2023
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf changed the title feat: sanitise log/console secrets bugfix: sanitise log/console secrets Jan 24, 2023
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf added bug Something isn't working and removed enhancement New feature or request labels Jan 24, 2023
Copy link
Contributor

@MiroslavRepka MiroslavRepka left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf.

In the description for #529 I mention that there is a possibility that other credentials are in logs, more concretely in command package (i.e. when retrying kubectl command with kubeconfig inside). Could you have a look and resolved it as well?

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor Author

LGTM, thanks...

🚀

...there is a possibility that other credentials are in logs ... when retrying kubectl command with kubeconfig inside...

as suggested, I tried fixing it at the source - the command package - as that's what all the kubectl methods are calling anyway.
let me know if I missed anything. @MiroslavRepka

Copy link
Contributor

@MiroslavRepka MiroslavRepka left a comment

Choose a reason for hiding this comment

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

Good job :)

I like the implementation, from a functional point of view, although, I left a comment, which we can discuss if needed.

internal/command/cmd.go Outdated Show resolved Hide resolved
internal/command/cmd.go Outdated Show resolved Hide resolved
internal/command/cmd.go Outdated Show resolved Hide resolved
internal/command/cmd.go Outdated Show resolved Hide resolved
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf added the question Further information is requested label Jan 26, 2023
* use ansibler image from master for now
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf marked this pull request as ready for review January 26, 2023 12:28
Copy link
Contributor

@MiroslavRepka MiroslavRepka left a comment

Choose a reason for hiding this comment

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

LGTM

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor Author

thanks for the reviews @MiroslavRepka 🚀

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf deleted the feature-fix-creds-in-conn-strings branch January 26, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go Pull requests that update Go code security Pertains project's security posture in some way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Secrets visible in logs
2 participants