Skip to content

Conversation

thriqon
Copy link
Contributor

@thriqon thriqon commented Nov 7, 2022

For our use case, we want do log additional information about a CSP report.

This change should be backwards-compatible:

  • The path field is new, but in structured logging and in JSON additional fields should not be a problem.
  • Client IP logging is disabled by default, as client IP is considerered sensitive data. Logging it should be opt-in.
  • In certain use cases, the query string or the fragment part (if transmitted by the browser) might contain sensitive data, which might no be needed to act on the report. Thus, it shouldn't be logged at all.

@jacobbednarz
Copy link
Owner

thanks for the PR @thriqon, this looks great. if you can address the failing CI checks, i'm happy to merge this in given it's backwards compatible.

net/netip is not present in 1.17
Package net/netip is missing in default go setup.
@thriqon
Copy link
Contributor Author

thriqon commented Nov 9, 2022

I've updated Go at three places, since net/netip is not available in 1.17.

@jacobbednarz
Copy link
Owner

i'm happy supporting 1.18+ (latest minus 2) so using that in CI seems reasonable to me 👍

@thriqon
Copy link
Contributor Author

thriqon commented Nov 9, 2022

Removed usage of io/ioutil, replaced with io and os as recommended in the docs.

@jacobbednarz
Copy link
Owner

LGTM to me @thriqon, thanks for your effort getting this one over the line!

@jacobbednarz jacobbednarz merged commit 8e6e1f8 into jacobbednarz:master Nov 9, 2022
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.

2 participants