Allow HTML tags/attributes to be whitelisted during Markdown processing

[Sherlouk]

Example: https://staging.swiftpackageindex.com/guykogus/CodableJSON (See 'Installation' section)

A relatively common feature on READMEs is the use of the 'details' HTML tag in order to make collapsible sections of documentation.

These are currently completely ignored by our rendering of READMEs.

Title

Contents with a predetermined title

No Title defaults to 'Details'

Follow-on from #410

[daveverwer]

Great catch. I hadn't seen this in my testing (and haven't seen it on GitHub either, which is strange as I look at a lot of package READMEs!)

Markdown processors should leave raw HTML unprocessed and just take it wholesale into the output, but it looks like the one we're using is replacing it and stripping raw HTML.

We should look if that's an option we can set, or a patch we can make to that parser as I'm not sure there's much we can do to fix this if it's not an option.

[Sherlouk]

This looks like it's the relevant part:
https://github.com/KristopherGBaker/libcmark_gfm/blob/master/Sources/libcmark_gfm/html.c#L228-L238 or https://github.com/KristopherGBaker/libcmark_gfm/blob/master/Sources/libcmark_gfm/html.c#L307-L326

So it seems like it might be in some sort of safe mode?

[Sherlouk]

Aye according to cmark docs:

/** Render raw HTML and unsafe links (`javascript:`, `vbscript:`,
 * `file:`, and `data:`, except for `image/png`, `image/gif`,
 * `image/jpeg`, or `image/webp` mime types).  By default,
 * raw HTML is replaced by a placeholder HTML comment. Unsafe
 * links are replaced by empty strings.
 */

this is what we currently have enabled, as it's the default.

https://github.com/KristopherGBaker/libcmark_gfm/blob/2abfe8c7d47557590aef8545d720c1297b359972/Sources/libcmark_gfm/include/cmark-gfm.h

[Sherlouk]

So I suppose it's a question of whether or not we're happy to drop it into unsafe mode - and if we do that whether or not we want to strip <script> tags etc ourselves?

[daveverwer]

Apologues for the delay responding to this, and thanks for investigating @Sherlouk!

Is there a tag/attribute whitelist option for unsafe mode in the GFM library? If there is, that's ideal and we'll just whitelist our standard tags and any raw HTML exceptions we find.

If not, we're in a relatively tricky situation as stripping unsafe HTML tags/attributes is almost impossible to get right. There's so much to do! 😬 You've got the obvious tags like script, iframe, embed, object but then you've also got to make sure you remove javascript:, data:, file:, references from everywhere they can be used img and a tags are the obvious ones, but I bet that's not a comprehensive list. Finally, you've got to remove all of the JavaScript-based attributes like onclick, onload, onfocus/onblur. It's effectively impossible to be comprehensive.

I have my fingers crossed for a whitelist 🤞

[Sherlouk]

From what I understand, it's not a whitelist.

You can either support no HTML, or all HTML. (or we need to fork the GFM library and add support for all the tags we want...)

[daveverwer]

Sven and I chatted about this today and we're not going to let this problem stop the README rollout, in fact, it's done (#855) 🚀

I'll retitle this bug as relating to the HTML whitelist, but the amount of READMEs that this breaks is not worth the trouble of all the potential security issues that unsafe mode brings.