Markdown rendering changes - stripping classes? #4345

Closed
opened 2025-11-02 05:47:08 -06:00 by GiteaMirror · 9 comments
Owner

Originally created by @cipherboy on GitHub (Nov 17, 2019).

  • Gitea version (or commit ref): 1.10.0
  • Git version: 2.20.1
  • Pandoc version: 2.5-2 (dpkg)
  • KaTeX version: 0.11.0
  • Operating system: Ubuntu 19.10
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
  • Log gist:

Nothing relevant in logs.

Description

I've installed a custom markdown rendering based on pandoc as such:

[markup.markdown]
ENABLED         = true
FILE_EXTENSIONS = .md,.markdown
RENDER_COMMAND  = pandoc -f markdown -t html --katex

This lets me add a custom header and render LaTeX in Markdown with KaTeX:

    <link rel="stylesheet" href="https://git.cipherboy.com/internal-custom/static/katex/katex.min.css" integrity="sha384-BdGj8xC2eZkQaxoQ8nSLefg4AV4/AwB3Fj+8SUSo7pnKP6Eoy18liIKTPn9oBYNG" crossorigin="anonymous">

    <!-- The loading of KaTeX is deferred to speed up page rendering -->
    <script defer src="https://git.cipherboy.com/internal-custom/static/katex/katex.min.js" integrity="sha384-JiKN5O8x9Hhs/UE5cT5AAJqieYlOZbGT3CHws/y97o3ty4R7/O5poG9F3JoiOYw1" crossorigin="anonymous"></script>

    <!-- To automatically render math in text elements, include the auto-render extension: -->
    <script defer src="https://git.cipherboy.com/internal-custom/static/katex/auto-render.min.js" integrity="sha384-kWPLUVMOks5AQFrykwIup5lo0m3iMkkHrD0uJ4H5cjeGihAutqP0yW0J6dpFiVkI" crossorigin="anonymous" onload="renderMathInElement(document.body);"></script>

Sometime recently (I remember it working early in 1.9.x series) this got broken. Looking at the source, it looks like classes on elements started getting stripped by gitea after pandoc got done rendering it.

For example, the browser gets sent source like:

...
<li><span>\{..., -16, -11, -6, -1, 4, 9, 14, ...\}</span></li>
<li><span>\{...,-7, -4, -1, 2, 5, 8, ...\}</span></li>
<li><span>\{-2, -1, 0, 1, 2, 3, 4, 5, 6\}</span></li>
<li><span>\{-1, 0, 1, 2, 3, 4, 5, 6, 7\}</span></li>
<li><span>\{-\sqrt{3}, \sqrt{3} \}</span></li>
...

However, running the pandoc command above on the server (on the same source file) gives:

...
<li><span class="math inline">\{..., -16, -11, -6, -1, 4, 9, 14, ...\}</span></li>
<li><span class="math inline">\{...,-7, -4, -1, 2, 5, 8, ...\}</span></li>
<li><span class="math inline">\{-2, -1, 0, 1, 2, 3, 4, 5, 6\}</span></li>
<li><span class="math inline">\{-1, 0, 1, 2, 3, 4, 5, 6, 7\}</span></li>
<li><span class="math inline">\{-\sqrt{3}, \sqrt{3} \}</span></li>
...

Which makes me think gitea changed something recently. This results in KaTeX not rendering anything, which means my Math+Markdown files are now broken.

Did something change? Perhaps more markdown sanitation was added recently?

Screenshots

Screenshot from 2019-11-17 08-44-23

Originally created by @cipherboy on GitHub (Nov 17, 2019). - Gitea version (or commit ref): 1.10.0 - Git version: 2.20.1 - Pandoc version: 2.5-2 (dpkg) - KaTeX version: 0.11.0 - Operating system: Ubuntu 19.10 - Database (use `[x]`): - [ ] PostgreSQL - [ ] MySQL - [ ] MSSQL - [x] SQLite - Can you reproduce the bug at https://try.gitea.io: - [ ] Yes (provide example URL) - [ ] No - [x] Not relevant -- server doesn't have pandoc+KaTeX rendering enabled. But if it did: https://try.gitea.io/test99112288/mathtest - Log gist: Nothing relevant in logs. ## Description I've installed a custom markdown rendering based on `pandoc` as such: ```ini [markup.markdown] ENABLED = true FILE_EXTENSIONS = .md,.markdown RENDER_COMMAND = pandoc -f markdown -t html --katex ``` This lets me add a custom header and render LaTeX in Markdown with KaTeX: ```html <link rel="stylesheet" href="https://git.cipherboy.com/internal-custom/static/katex/katex.min.css" integrity="sha384-BdGj8xC2eZkQaxoQ8nSLefg4AV4/AwB3Fj+8SUSo7pnKP6Eoy18liIKTPn9oBYNG" crossorigin="anonymous"> <!-- The loading of KaTeX is deferred to speed up page rendering --> <script defer src="https://git.cipherboy.com/internal-custom/static/katex/katex.min.js" integrity="sha384-JiKN5O8x9Hhs/UE5cT5AAJqieYlOZbGT3CHws/y97o3ty4R7/O5poG9F3JoiOYw1" crossorigin="anonymous"></script> <!-- To automatically render math in text elements, include the auto-render extension: --> <script defer src="https://git.cipherboy.com/internal-custom/static/katex/auto-render.min.js" integrity="sha384-kWPLUVMOks5AQFrykwIup5lo0m3iMkkHrD0uJ4H5cjeGihAutqP0yW0J6dpFiVkI" crossorigin="anonymous" onload="renderMathInElement(document.body);"></script> ``` Sometime recently (I remember it working early in 1.9.x series) this got broken. Looking at the source, it looks like classes on elements started getting stripped by gitea after `pandoc` got done rendering it. For example, the browser gets sent source like: ```html ... <li><span>\{..., -16, -11, -6, -1, 4, 9, 14, ...\}</span></li> <li><span>\{...,-7, -4, -1, 2, 5, 8, ...\}</span></li> <li><span>\{-2, -1, 0, 1, 2, 3, 4, 5, 6\}</span></li> <li><span>\{-1, 0, 1, 2, 3, 4, 5, 6, 7\}</span></li> <li><span>\{-\sqrt{3}, \sqrt{3} \}</span></li> ... ``` However, running the `pandoc` command above on the server (on the same source file) gives: ```html ... <li><span class="math inline">\{..., -16, -11, -6, -1, 4, 9, 14, ...\}</span></li> <li><span class="math inline">\{...,-7, -4, -1, 2, 5, 8, ...\}</span></li> <li><span class="math inline">\{-2, -1, 0, 1, 2, 3, 4, 5, 6\}</span></li> <li><span class="math inline">\{-1, 0, 1, 2, 3, 4, 5, 6, 7\}</span></li> <li><span class="math inline">\{-\sqrt{3}, \sqrt{3} \}</span></li> ... ``` Which makes me think gitea changed something recently. This results in KaTeX not rendering anything, which means my Math+Markdown files are now broken. Did something change? Perhaps more markdown sanitation was added recently? ## Screenshots ![Screenshot from 2019-11-17 08-44-23](https://user-images.githubusercontent.com/914030/69008250-7bd97200-0916-11ea-88e5-2822555be501.png)
Author
Owner

@guillep2k commented on GitHub (Nov 18, 2019):

Currently, output from all renderers is passed through a post-processor and an HTML sanitizer:

https://github.com/go-gitea/gitea/blob/f8bd90ba60b0c362d3e39ddf702cac0e0df2b0ab/modules/markup/markup.go#L83-L92

The sanitizer rules are getting in your way. There's no way ATM of bypassing or tailoring those rules.

@guillep2k commented on GitHub (Nov 18, 2019): Currently, output from all renderers is passed through a post-processor and an HTML sanitizer: https://github.com/go-gitea/gitea/blob/f8bd90ba60b0c362d3e39ddf702cac0e0df2b0ab/modules/markup/markup.go#L83-L92 The sanitizer rules are getting in your way. There's no way ATM of bypassing or tailoring those rules.
Author
Owner

@guillep2k commented on GitHub (Nov 18, 2019):

I don't know in what version that behavior could have changed, though.

@guillep2k commented on GitHub (Nov 18, 2019): I don't know in what version that behavior could have changed, though.
Author
Owner

@guillep2k commented on GitHub (Nov 18, 2019):

May be options to bypass those steps could be added to the renderers' configuration?

@guillep2k commented on GitHub (Nov 18, 2019): May be options to bypass those steps could be added to the renderers' configuration?
Author
Owner

@cipherboy commented on GitHub (Nov 18, 2019):

I don't know in what version that behavior could have changed, though.

Ah no, you're right and I'm wrong. I pulled 1.9.1, 1.9.0, and 1.8.3 and all had the same behavior as 1.10.0. I must've been misremembering the order I did the migration in. I thought I went gogs -> gitea and then added the custom renderer but I must've done the reverse. Sorry for the noise!

May be options to bypass those steps could be added to the renderers' configuration?

That'd be fine with me (UNSAFE_NO_SANITIZE or some such). My use case for gitea being is as a locked-down git forge for myself and very few others, with both DISABLE_REGISTRATION = true and REQUIRE_SIGNIN_VIEW = true. If someone uploads malicious Markdown, they're probably pranking me.

Alternatively, what about something like:

diff --git a/modules/markup/sanitizer.go b/modules/markup/sanitizer.go
index f873e8105..66bf1b60f 100644
--- a/modules/markup/sanitizer.go
+++ b/modules/markup/sanitizer.go
@@ -48,6 +48,9 @@ func ReplaceSanitizer() {
 
        // Allow keyword markup
        sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^` + keywordClass + `$`)).OnElements("span")
+
+       // Allow KaTeX markup from pandoc
+       sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^(math\s*|inline\s*|display\s*){0,3}$`)).OnElements("span")
 }
 
 // Sanitize takes a string that contains a HTML fragment or document and applies policy whitelist.

That seems simpler: you're letting through a very small subset of classes and in the default installation, nothing will happen because you're not shipping KaTeX and you're not shipping the Pandoc render. The owner would have to manually add KaTeX rendering (scripts + css + ...) and modify the config to switch to Pandoc to hit this in most cases.

Thought?

@cipherboy commented on GitHub (Nov 18, 2019): > I don't know in what version that behavior could have changed, though. Ah no, you're right and I'm wrong. I pulled 1.9.1, 1.9.0, and 1.8.3 and all had the same behavior as 1.10.0. I must've been misremembering the order I did the migration in. I thought I went gogs -> gitea and then added the custom renderer but I must've done the reverse. Sorry for the noise! > May be options to bypass those steps could be added to the renderers' configuration? That'd be fine with me (`UNSAFE_NO_SANITIZE` or some such). My use case for gitea being is as a locked-down git forge for myself and very few others, with both `DISABLE_REGISTRATION = true` and `REQUIRE_SIGNIN_VIEW = true`. If someone uploads malicious Markdown, they're probably pranking me. Alternatively, what about something like: ```diff diff --git a/modules/markup/sanitizer.go b/modules/markup/sanitizer.go index f873e8105..66bf1b60f 100644 --- a/modules/markup/sanitizer.go +++ b/modules/markup/sanitizer.go @@ -48,6 +48,9 @@ func ReplaceSanitizer() { // Allow keyword markup sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^` + keywordClass + `$`)).OnElements("span") + + // Allow KaTeX markup from pandoc + sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^(math\s*|inline\s*|display\s*){0,3}$`)).OnElements("span") } // Sanitize takes a string that contains a HTML fragment or document and applies policy whitelist. ``` That seems simpler: you're letting through a very small subset of classes and in the default installation, nothing will happen because you're not shipping KaTeX and you're not shipping the Pandoc render. The owner would have to manually add KaTeX rendering (scripts + css + ...) and modify the config to switch to Pandoc to hit this in most cases. Thought?
Author
Owner

@lunny commented on GitHub (Nov 19, 2019):

Or we could add custom regexp express on third-party external renderer configuration so that user could define themselves.

@lunny commented on GitHub (Nov 19, 2019): Or we could add custom regexp express on third-party external renderer configuration so that user could define themselves.
Author
Owner

@cipherboy commented on GitHub (Nov 19, 2019):

That sounds like the best candidate. May I take a shot at implementing that?

@cipherboy commented on GitHub (Nov 19, 2019): That sounds like the best candidate. May I take a shot at implementing that? >
Author
Owner

@guillep2k commented on GitHub (Nov 19, 2019):

That sounds like the best candidate. May I take a shot at implementing that?

@cipherboy Of course! You can check modules/markup/sanitizer.go. Rules are added at ReplaceSanitizer().

@guillep2k commented on GitHub (Nov 19, 2019): > > That sounds like the best candidate. May I take a shot at implementing that? > […](#) @cipherboy Of course! You can check `modules/markup/sanitizer.go`. Rules are added at `ReplaceSanitizer()`.
Author
Owner

@cipherboy commented on GitHub (Nov 19, 2019):

OK, I have a working draft on my fork.

Is it possible to run tests without drone? It doesn't work on my system as it seems to require Docker, which doesn't work very well (last time I tried pulling a Docker container it crashed the Docker daemon...). Podman works better but it doesn't appear that drone CLI has support for podman yet.

@cipherboy commented on GitHub (Nov 19, 2019): OK, I have a [working draft on my fork](https://github.com/cipherboy/gitea/tree/markdown-sanitizer-config). Is it possible to run tests without `drone`? It doesn't work on my system as it seems to require Docker, which doesn't work very well (last time I tried pulling a Docker container it crashed the Docker daemon...). Podman works better but it doesn't appear that [`drone` CLI has support for `podman` yet](https://github.com/drone/drone-runtime/issues/58).
Author
Owner

@guillep2k commented on GitHub (Nov 19, 2019):

You certainly can run partial (but meaningful) tests with:

make test
make test-sqlite

Once you're satisfied, you can run:

make lint
make revive
@guillep2k commented on GitHub (Nov 19, 2019): You certainly can run partial (but meaningful) tests with: ``` make test make test-sqlite ``` Once you're satisfied, you can run: ``` make lint make revive ```
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#4345