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

chore(logging): log all headers but truncate or redact sensitive values #1646

Closed
wants to merge 6 commits into from

Conversation

iainsproat
Copy link
Contributor

@iainsproat iainsproat commented Jun 22, 2023

Description & motivation

Addresses #1644

Previously we would remove headers which may contain sensitive content. This was frustrating as we could not determine if a header was correctly included, or we could not determine if the value was as expected. While we do not wish to log the entire value it is appropriate in many cases to log the length of the value or a truncated portion of the value.

Changes:

  • uses pino's redact property with a custom censor function
  • all censored tokens have their original length printed
  • authorization bearer tokens have the first 10 digits printed, to allow for identifying token IDs when debugging

To-do before merge:

Screenshots:

Validation of changes:

Checklist:

  • My pull request follows the guidelines in the Contributing guide?
  • My pull request does not duplicate any other open Pull Requests for the same update/change?
  • My commits are related to the pull request and do not amend unrelated code or documentation.
  • My code follows a similar style to existing code.
  • I have added appropriate tests.
  • I have updated or added relevant documentation.

References

@iainsproat iainsproat marked this pull request as ready for review June 22, 2023 10:29
@iainsproat iainsproat requested a review from gjedlicska June 22, 2023 11:40
@iainsproat
Copy link
Contributor Author

Amendments have been tested and will follow the following pattern:

INFO: graphql response {"req":{"id":"d61f77b0-cd1a-44fb-810c-e931c7eaa016","method":"POST","headers":{"host":"127.0.0.1:59666","accept-encoding":"gzip, deflate","user-agent":"node-superagent/3.8.3","authorization":"Bearer 99e6a3d89e[TRUNCATED(original_length:48)]","content-type":"application/json","content-length":"98","connection":"close","x-request-id":"d61f77b0-cd1a-44fb-810c-e931c7eaa016"}},"authContext":{"auth":true,"userId":"33a3913421","role":"server:admin","scopes":["server:setup","streams:read","streams:write","users:read","users:email","tokens:write","tokens:read","profile:read","profile:email"]},"component":"graphql","graphql_operation_kind":"query","graphql_query":"query { stream(id:\"b340641aca\") { branch( name: \"main\" ) { name description } } } ","graphql_operation_value":"GQL query stream","grqphql_operation_name":"GQL stream","actionName":"query stream"}
INFO: request completed {"req":{"id":"d61f77b0-cd1a-44fb-810c-e931c7eaa016","method":"POST","path":"/graphql","headers":{"host":"127.0.0.1:59666","accept-encoding":"gzip, deflate","user-agent":"node-superagent/3.8.3","authorization":"Bearer 99e6a3d89e[TRUNCATED(original_length:49)]","content-type":"application/json","content-length":"98","connection":"close","x-request-id":"d61f77b0-cd1a-44fb-810c-e931c7eaa016"}},"res":{"statusCode":200},"responseTime":71}

@iainsproat iainsproat changed the title chore(logging): log all headers but truncate or redact chore(logging): log all headers but truncate or redact sensitive values Jun 22, 2023
@iainsproat iainsproat mentioned this pull request Jul 11, 2023
6 tasks
@iainsproat
Copy link
Contributor Author

Superseded

@iainsproat iainsproat closed this Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant