KEMBAR78
[bugfix] Ensures that he API Token in clear text is never present in the error stack trace when set in the URL by nickfloyd · Pull Request #1562 · octokit/octokit.rb · GitHub
Skip to content

Conversation

nickfloyd
Copy link
Contributor

Resolves #1559


Behavior

Before the change?

  • Prior to the change, the api_token that was passed via URI (currently the only way the GitHub REST API accepts this value) was being persisted and sent to STDOUT in the stack trace when errors occurred.

After the change?

  • The value of the key from the URL now displays as http://127.0.0.1:8989/setup/api/settings?api_key=(redacted)

Other information

I added tests for the private method.

I have mixed feelings about testing this method from this class like this due to potentially breaking encapsulation, but given the nature of this method and the fact that it is meant to keep secrets out of logs.

I feel like it is worth it and will help us know if something changes in this area.


Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

@nickfloyd nickfloyd added the Type: Bug Something isn't working as documented label Apr 6, 2023
@nickfloyd nickfloyd marked this pull request as draft April 6, 2023 19:21
@nickfloyd nickfloyd marked this pull request as ready for review April 6, 2023 20:07
@nickfloyd nickfloyd requested a review from kfcampbell April 6, 2023 20:08
Copy link
Contributor

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

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

One day our API will accept this in a more secure fashion. Until then, this is a good alternative. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Bug Something isn't working as documented

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[BUG]: Octokit.rb exceptions leak api_key and secret settings

3 participants