Some security and reliability improvements #35
Conversation
The previous default is a huge security hazard...
By default, scripts are blocked because there is hardly any legitimate reason for allowing them. External content such as images are not uncommon, so these are allowed by default. Those who don't want any external network activity when rendering markdown can block them via environment variables.
Without a specific version in package.json, this could break without warning in the future...
- If a server was running, forward the stdin stream. - If a new tab is opened, re-use the last rendered markdown.
|
This fully addresses #33 correct? |
|
I know for sure there are users who rely on being able to hit the server from non-localhost addresses - mainly for when they're previewing markdown on an SSHed-in server, ala instant-markdown/vim-instant-markdown#92 With that in mind, would you mind also putting together a small PR on https://github.com/suan/vim-instant-markdown to add the new env vars as vim settings, which will get passed on to instant-markdown-d on startup? Otherwise we'd be making life hard for them as they would basically have to set the envars globally in their The rest of the change looks good - and thanks again! |
g:instant_markdown_open_to_the_world g:instant_markdown_allow_unsafe_content g:instant_markdown_allow_external_content See instant-markdown/instant-markdown-d#35
|
@Rob--W I just noticed another issue - this change removes the default behavior of closing the tab after closing the file in Vim in favor of displaying a red message on the page and keeping it open. I've never heard a feature request for this and neither does it suit my workflow, so I'd like to keep the existing behavior. Would you mind removing that part from this PR or at least extracting it into its own PR so I can learn more about your use case for it? Thanks. |
Previously the tight CSP would prevent the preview from working in Chrome because the stylesheets and WebSocket was blocked. And if unsafe content was allowed, then the variable to block external content did not work (INSTANT_MARKDOWN_BLOCK_EXTERNAL=1). Both issues have been fixed.
|
@suan It was not an intentional change, I've fixed it. |
This PR fixes a security vulnerability and improves the reliability of rendering. See the individual commit messages and the README for details.
I suggest to bump it to at least 0.2.0 because the change may be backwards-incompatible for those who rely on scripts being executed in markdown (?!!?!), or for those who rely on the previewer being accessible over non-localhost.