Skip to content

Hide the password to prevent it being accidently displayed #2

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

Merged
merged 2 commits into from
Oct 19, 2024

Conversation

Baptouuuu
Copy link
Member

Problem

While testing innmind/s3 an error occurred and displayed the bucket object along its url. The url contained the bucket password. Since this is a sensitive value it should never be exposed.

Solution

Wrap the password string inside a SensitiveParameterValue so tools like var_dump, symfony/var-dumper or loggers know the value should not be displayed.

Note

The password is still exposed when building the Password object as a parameter or the exception. The exception is not modified for backward compatibility. The parameters are not flagged as SensitiveParameter to help developers debug their code, this is relatively safe as most Urls should be built at compile time (where any exception should not be displayed to end users) and the ones built at runtime should be wrapped inside a try/catch (and once again not expose the exception message).

These decisions may change in a future major version if feedback suggest it's better to hide everything.

@Baptouuuu Baptouuuu added the bug label Oct 19, 2024
@Baptouuuu Baptouuuu self-assigned this Oct 19, 2024
Copy link

codecov bot commented Oct 19, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@Baptouuuu Baptouuuu merged commit 1134de0 into develop Oct 19, 2024
23 checks passed
@Baptouuuu Baptouuuu deleted the sensitive-parameter branch October 19, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant