search

google / docsy

A set of Hugo doc templates for launching open source content.
https://docsy.dev
Apache License 2.0
2.62k stars 904 forks source link

How to use Goldmark renderer without setting unsafe to true? #323

Open andreamussap opened 4 years ago

andreamussap commented 4 years ago

Hello. On https://github.com/Axway/axway-open-docs project, we've been using Hugo Blackfriday markdown parser, and now we want to update it to Goldmark, see #1047.

However, I've read on Docsy docs that to use Goldmark, I'll have to set the unsafe property to true: unsafe = true.

Doing that will allow someone to add HTML code into our markdown files, which can cause a security issue. However, if we don't turn the unsafe on, the content/en/_index.html page won't render.

So, I'd like to ask you if there's a way to make the content/en/_index.html renders correctly having unsafe = false, and if not, are you're planning a fix for that?

Thanks, Andrea.

LisaFC commented 4 years ago

Hi Andrea, I'll take a look. The index.html page was built for Blackfriday, which always allowed raw HTML, so we had to add the unsafe requirement just so the switch to Goldmark wouldn't break multiple existing sites. I'll see if there's a way to redo it just with Markdown (though do you have a particular security concern around raw HTML blocks? They're permitted in most flavours of Markdown including CommonMark and I tend to use them for things like large tables that are extremely difficult to maintain in Markdown markup - unless you're getting enormous PRs it should be possible to see at review time if people are submitting anything weird)

andreamussap commented 4 years ago

Hi Lisa, sorry my late response. RE do you have a particular security concern around raw HTML blocks --> Yes. This was highlighted by the security team in my company. Allowing HTML in the markdown files there's a risk of someone embed XSS attack in the code/content. See this page that has some Javascript added. In this case it's an obvious alert box, but it could be a slick attack:

https://deploy-preview-464--axway-open-docs.netlify.com/docs/apim_installation/apigw_containers/docker_scripts_prereqs/

If unsafe is set to False it will prevent known HTML that can execute javascript, right?

LisaFC commented 4 years ago

If unsafe is set to False it literally just omits anything marked up as HTML from the generated site - you get this if you look at the source code:

<!-- raw HTML omitted -->

And yes, it does mean you can't accidentally review and merge in a PR that includes malicious JS (well, you can, but it won't end up in the site). It does however make some site formatting a bit tricky.

I think I can fix the shortcodes (the blocks with content below the landing page image) so that they'll work in safe or unsafe mode, though the nice buttons and things at the top below the title are just HTML so won't show up. I may be able to turn them into a shortcode but I'm not sure....

LisaFC commented 4 years ago

Though also if I change the shortcodes I will break all existing site landing pages on updating to the latest theme, as users would need to switch from {{% %}} to {{< >}}. Would need to make sure everyone knew about it. Hmm.