search

go-pkgz / auth

Authenticator via oauth2, direct, email and telegram
https://go-pkgz.umputun.dev/auth/
MIT License
1.07k stars 84 forks source link

remove bluemonday as an unreliable dependency #183

Closed david-bezero closed 10 months ago

david-bezero commented 1 year ago

It seems the "bluemonday" dependency (used for HTML sanitisation) has a policy of unilaterally deleting their old versions shortly after releasing a new version:

Version: v1.0.26 Latest Published: Oct 12, 2023

As of today (Oct 16), we see:

go: warning: github.com/microcosm-cc/bluemonday@v1.0.25: retracted by module author: Retract older versions as only latest is to be depended upon go: to switch to the latest unretracted version, run: go get github.com/microcosm-cc/bluemonday@latest

We have also seen the same happen on the 22nd of July 2022.

This is potentially very problematic, as it means old versions of applications cannot be rebuilt.

Since this dependency is only being used in a couple of locations, it shouldn't be difficult to remove it entirely, sidestepping the version management issue.

The locations it's currently used:

  1. verify.sanitise
  2. custom_server

Both of these seem to be incorrect uses of HTML sanitisation anyway: the data is provided to JSON structures (the second one is being built manually and does need some escaping; currently it would actually break / be exploitable if the user had a " or \ in their user ID, but that's perhaps a separate issue). The server doesn't need to strip HTML tags in the first place, as handling special characters is the concern of whatever later displays this data on a page (and should be achieved by escaping rather than stripping).

umputun commented 1 year ago

I agree; this is genuinely troublesome. I have opened a discussion and asked the author to reconsider the decision. Feel free to add more details and reasoning.

david-bezero commented 1 year ago

to be clear: though this issue was the trigger for me investigating, the result of my investigation is that bluemonday is not required here anyway and should be removed regardless (as in the PR I created).