MarkdownAIO - #82

Closed
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO
Closed

MarkdownAIO#82
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO

Conversation

@AnnMarieW

@AnnMarieWAnnMarieW commented Jan 31, 2022

Copy link
Copy Markdown
Collaborator

Adds the new component MarkdownAIO

See the live demo

MarkdownAIO is a Dash feature that allows you to write Dash Apps as Markdown files. Simply pass in a Markdown file and MarkdownAIO will return a set of components with the option to display and/or execute code blocks. So it’s part:

  • Documentation helper
  • Markdown / Text authoring tool
  • Easy way to bring a Markdown file into an app without doing open('file.md')

It's also compatible with the pages/ api. The online docs for dash-labs is a multi-page app made with pages/ and each page is a Markdown file displayed using MarkdownAIO.


Here is a summary of the TODOs and questions:

  • To further reduce security risk with exec, check for local files only. Ensure that the file is within a certain directory, and by default that should be the parent directory of the main app but maybe we could create a way to override that. Similar to /assets?
    Update - require full path from pages/

  • css:

    • create a MarkdownAIO stylesheet?
    • eliminated dbc dependency (replace Rows and Cols with inline css)
    • document the default style in subcomponents. Or better yet - use a stylesheet?
      Status: inprogress
  • see todos in pages.py in _register_page_from_markdown_file()

  • [x ] use UUID for clipboard ID?

  • add ability to change defaults "globally" so it doesn't have to be done for each MarkdownAIO() instance.

  • add ability to embed another file within the Markdown file. Use case being, ability to keep the python app code in a separate file so you can run it individually. We could integrate jinja in here perhaps…: {% include code.py %}

  • Add Markdown files to hot reload in dash. That way users can have the same hot-reloading dev experience when working in markdown

  • if code block is not executed don't register callbacks and layout? Goal it to reduce the number of id clashes.

  • Need to remove if __name__ == "__main__": ... from the code blocks?

  • refactor _update_props() (It works, but it's kinda ugly)

  • rewrite the _remove_app_instance() to use the AST module

@AnnMarieW
AnnMarieW marked this pull request as ready for review January 31, 2022 17:21

@chriddypchriddyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking awesome! several small changes requested. once those are made, i'll take a deeper look at the docs 🙂

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated

- `exec_code` (boolean; default False):
If `True`, code blocks will be executed. This may also be set within the code block with the comment
# exec-code-true or # exec-code-false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe where this comment should exist. perhaps include a full example with the triple backticks

@AnnMarieWAnnMarieWFeb 2, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can actually be anywhere in the code block on a comment line. It's just that the prop won't be won't be displayed if it's included on the first line with the back ticks. Since there are 7 props that this applies to, maybe it would be better to have the full example in the docs instead of here.

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/plugins/pages.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
side_by_side: True
---
"""
import frontmatter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious to see how this library parsed YAML, given my exec / eval concerns when writing the code fence parser.

It looks like frontmatter depends on PyYAML's safe mode: https://github.com/eyeseast/python-frontmatter/blob/1c49958fdd504691fc842045425ea298316aa643/frontmatter/default_handlers.py#L224-L228

So then looking into PyYAML, I see some stuff like:

Note that the ability to construct an arbitrary Python object may be dangerous if you receive a YAML document from an untrusted source such as the Internet. The function yaml.safe_load limits this ability to simple Python objects like integers or lists.

from https://pyyaml.org/wiki/PyYAMLDocumentation

Then googled around yaml.safe_load and came across yaml/pyyaml#420

They fixed the issue which is great but the fact that an issue happened in the first place makes me feel a little concerned that there could be more issues lurking in that library.

So I might consider supporting a smaller subset of the YAML library and doing our own parsing with JSON, similar to what we did with the code fences.

I'll keep thinking a bit about this

Comment threaddash_labs/plugins/pages.py Outdated
<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to put this in the stylesheet that we document

Comment threaddash_labs/plugins/pages.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
if "app.layout" in code:
code = code.replace("app.layout", "layout")
if "layout" in code:
exec(code, scope)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling like we should lock this down a bit more and exclude folks from using exec within callbacks.
for users, this means:

  • MarkdownAIO can only be called outside of callbacks, meaning when the app starts up
  • All of the MarkdownAIO files would need to be written in advance of the app starting up

so this wouldn't be allowed:
pages/report.py

dash.register_page(__name__)
def layout():
return MarkdownAIO('report.md', exec_code=True)

But this would be allowed:
pages/report.py

dash.register_page(__name__)
layout = MarkdownAIO('report.md', exec_code=True)

I can't think of many examples where this would be insufficient and it would prevent users from seemingly innocent but very insecure code like:

template_report.md

# Report for {customer}
`` `python
dcc.Graph(figure=px.scatter(...))
`` `

app.py

app.layout=html.Div([
dcc.Dropdown(id='customer', options=['Acme', 'City']),
html.Div(id='content')
])
@callback(Output('content', 'children'), Input('customer', 'value'))defupdate(customer):
withopen('template_report.md', 'r') asf:
content=f.read()
customer_report=content.replace(customer=customer)
withopen('customer_report.md', 'w') asf:
f.write(customer_report)
returnMarkdownAIO('customer_report.md', exec_code=True)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to prevent exec from running in a callback would be to do:

def_exec(*args, **kwargs):
# flask raises an error if flask.request.path is called outside of a requesttry:
flask.request.pathexceptRuntimeErrorase:
raiseException('MarkdownAIO is being called with `exec_code=True` within a callback or flask request. This isn\'t supported. MarkdownAIO with exec can only be called when the app is starting')
returnexec(*args, **kwargs)

@AnnMarieWAnnMarieWFeb 15, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried this it prevented all exec s in the app. Checking for flask.has_request_context() worked though.

Will this be overly restrictive with a multi-page app using path variables and query strings? Also, many pages are functions even if there are no variables passed to the layout.

Update: This only works if it's called from a .py file. If the MarkdownAIO is inside a callback in a .md file, then
the flask.has_request_context() is False

new format for props in code blocks
use ast to remove app instance
removed dbc dependency
@AnnMarieWAnnMarieW mentioned this pull request Feb 10, 2022
12 tasks
@AnnMarieW

Copy link
Copy Markdown
CollaboratorAuthor

This was a cool project but did't make it across the finish line because there were concerns about executing the code securely.

There are some other community project that are good for project that are a mix of code and markdown text. See:

https://github.com/snehilvj/markdown2dash
https://github.com/snehilvj/dmc-docs
https://github.com/emilhe/dash-down

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AnnMarieW@chriddyp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

MarkdownAIO - #82

Closed
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO
Closed

MarkdownAIO#82
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO

Conversation

@AnnMarieW

@AnnMarieWAnnMarieW commented Jan 31, 2022

Copy link
Copy Markdown
Collaborator

Adds the new component MarkdownAIO

See the live demo

MarkdownAIO is a Dash feature that allows you to write Dash Apps as Markdown files. Simply pass in a Markdown file and MarkdownAIO will return a set of components with the option to display and/or execute code blocks. So it’s part:

  • Documentation helper
  • Markdown / Text authoring tool
  • Easy way to bring a Markdown file into an app without doing open('file.md')

It's also compatible with the pages/ api. The online docs for dash-labs is a multi-page app made with pages/ and each page is a Markdown file displayed using MarkdownAIO.


Here is a summary of the TODOs and questions:

  • To further reduce security risk with exec, check for local files only. Ensure that the file is within a certain directory, and by default that should be the parent directory of the main app but maybe we could create a way to override that. Similar to /assets?
    Update - require full path from pages/

  • css:

    • create a MarkdownAIO stylesheet?
    • eliminated dbc dependency (replace Rows and Cols with inline css)
    • document the default style in subcomponents. Or better yet - use a stylesheet?
      Status: inprogress
  • see todos in pages.py in _register_page_from_markdown_file()

  • [x ] use UUID for clipboard ID?

  • add ability to change defaults "globally" so it doesn't have to be done for each MarkdownAIO() instance.

  • add ability to embed another file within the Markdown file. Use case being, ability to keep the python app code in a separate file so you can run it individually. We could integrate jinja in here perhaps…: {% include code.py %}

  • Add Markdown files to hot reload in dash. That way users can have the same hot-reloading dev experience when working in markdown

  • if code block is not executed don't register callbacks and layout? Goal it to reduce the number of id clashes.

  • Need to remove if __name__ == "__main__": ... from the code blocks?

  • refactor _update_props() (It works, but it's kinda ugly)

  • rewrite the _remove_app_instance() to use the AST module

@AnnMarieW
AnnMarieW marked this pull request as ready for review January 31, 2022 17:21

@chriddypchriddyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking awesome! several small changes requested. once those are made, i'll take a deeper look at the docs 🙂

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated

- `exec_code` (boolean; default False):
If `True`, code blocks will be executed. This may also be set within the code block with the comment
# exec-code-true or # exec-code-false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe where this comment should exist. perhaps include a full example with the triple backticks

@AnnMarieWAnnMarieWFeb 2, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can actually be anywhere in the code block on a comment line. It's just that the prop won't be won't be displayed if it's included on the first line with the back ticks. Since there are 7 props that this applies to, maybe it would be better to have the full example in the docs instead of here.

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/plugins/pages.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
side_by_side: True
---
"""
import frontmatter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious to see how this library parsed YAML, given my exec / eval concerns when writing the code fence parser.

It looks like frontmatter depends on PyYAML's safe mode: https://github.com/eyeseast/python-frontmatter/blob/1c49958fdd504691fc842045425ea298316aa643/frontmatter/default_handlers.py#L224-L228

So then looking into PyYAML, I see some stuff like:

Note that the ability to construct an arbitrary Python object may be dangerous if you receive a YAML document from an untrusted source such as the Internet. The function yaml.safe_load limits this ability to simple Python objects like integers or lists.

from https://pyyaml.org/wiki/PyYAMLDocumentation

Then googled around yaml.safe_load and came across yaml/pyyaml#420

They fixed the issue which is great but the fact that an issue happened in the first place makes me feel a little concerned that there could be more issues lurking in that library.

So I might consider supporting a smaller subset of the YAML library and doing our own parsing with JSON, similar to what we did with the code fences.

I'll keep thinking a bit about this

Comment threaddash_labs/plugins/pages.py Outdated
<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to put this in the stylesheet that we document

Comment threaddash_labs/plugins/pages.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
if "app.layout" in code:
code = code.replace("app.layout", "layout")
if "layout" in code:
exec(code, scope)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling like we should lock this down a bit more and exclude folks from using exec within callbacks.
for users, this means:

  • MarkdownAIO can only be called outside of callbacks, meaning when the app starts up
  • All of the MarkdownAIO files would need to be written in advance of the app starting up

so this wouldn't be allowed:
pages/report.py

dash.register_page(__name__)
def layout():
return MarkdownAIO('report.md', exec_code=True)

But this would be allowed:
pages/report.py

dash.register_page(__name__)
layout = MarkdownAIO('report.md', exec_code=True)

I can't think of many examples where this would be insufficient and it would prevent users from seemingly innocent but very insecure code like:

template_report.md

# Report for {customer}
`` `python
dcc.Graph(figure=px.scatter(...))
`` `

app.py

app.layout=html.Div([
dcc.Dropdown(id='customer', options=['Acme', 'City']),
html.Div(id='content')
])
@callback(Output('content', 'children'), Input('customer', 'value'))defupdate(customer):
withopen('template_report.md', 'r') asf:
content=f.read()
customer_report=content.replace(customer=customer)
withopen('customer_report.md', 'w') asf:
f.write(customer_report)
returnMarkdownAIO('customer_report.md', exec_code=True)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to prevent exec from running in a callback would be to do:

def_exec(*args, **kwargs):
# flask raises an error if flask.request.path is called outside of a requesttry:
flask.request.pathexceptRuntimeErrorase:
raiseException('MarkdownAIO is being called with `exec_code=True` within a callback or flask request. This isn\'t supported. MarkdownAIO with exec can only be called when the app is starting')
returnexec(*args, **kwargs)

@AnnMarieWAnnMarieWFeb 15, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried this it prevented all exec s in the app. Checking for flask.has_request_context() worked though.

Will this be overly restrictive with a multi-page app using path variables and query strings? Also, many pages are functions even if there are no variables passed to the layout.

Update: This only works if it's called from a .py file. If the MarkdownAIO is inside a callback in a .md file, then
the flask.has_request_context() is False

new format for props in code blocks
use ast to remove app instance
removed dbc dependency
@AnnMarieWAnnMarieW mentioned this pull request Feb 10, 2022
12 tasks
@AnnMarieW

Copy link
Copy Markdown
CollaboratorAuthor

This was a cool project but did't make it across the finish line because there were concerns about executing the code securely.

There are some other community project that are good for project that are a mix of code and markdown text. See:

https://github.com/snehilvj/markdown2dash
https://github.com/snehilvj/dmc-docs
https://github.com/emilhe/dash-down

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AnnMarieW@chriddyp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

MarkdownAIO - #82

Closed
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO
Closed

MarkdownAIO#82
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO

Conversation

@AnnMarieW

@AnnMarieWAnnMarieW commented Jan 31, 2022

Copy link
Copy Markdown
Collaborator

Adds the new component MarkdownAIO

See the live demo

MarkdownAIO is a Dash feature that allows you to write Dash Apps as Markdown files. Simply pass in a Markdown file and MarkdownAIO will return a set of components with the option to display and/or execute code blocks. So it’s part:

  • Documentation helper
  • Markdown / Text authoring tool
  • Easy way to bring a Markdown file into an app without doing open('file.md')

It's also compatible with the pages/ api. The online docs for dash-labs is a multi-page app made with pages/ and each page is a Markdown file displayed using MarkdownAIO.


Here is a summary of the TODOs and questions:

  • To further reduce security risk with exec, check for local files only. Ensure that the file is within a certain directory, and by default that should be the parent directory of the main app but maybe we could create a way to override that. Similar to /assets?
    Update - require full path from pages/

  • css:

    • create a MarkdownAIO stylesheet?
    • eliminated dbc dependency (replace Rows and Cols with inline css)
    • document the default style in subcomponents. Or better yet - use a stylesheet?
      Status: inprogress
  • see todos in pages.py in _register_page_from_markdown_file()

  • [x ] use UUID for clipboard ID?

  • add ability to change defaults "globally" so it doesn't have to be done for each MarkdownAIO() instance.

  • add ability to embed another file within the Markdown file. Use case being, ability to keep the python app code in a separate file so you can run it individually. We could integrate jinja in here perhaps…: {% include code.py %}

  • Add Markdown files to hot reload in dash. That way users can have the same hot-reloading dev experience when working in markdown

  • if code block is not executed don't register callbacks and layout? Goal it to reduce the number of id clashes.

  • Need to remove if __name__ == "__main__": ... from the code blocks?

  • refactor _update_props() (It works, but it's kinda ugly)

  • rewrite the _remove_app_instance() to use the AST module

@AnnMarieW
AnnMarieW marked this pull request as ready for review January 31, 2022 17:21

@chriddypchriddyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking awesome! several small changes requested. once those are made, i'll take a deeper look at the docs 🙂

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated

- `exec_code` (boolean; default False):
If `True`, code blocks will be executed. This may also be set within the code block with the comment
# exec-code-true or # exec-code-false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe where this comment should exist. perhaps include a full example with the triple backticks

@AnnMarieWAnnMarieWFeb 2, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can actually be anywhere in the code block on a comment line. It's just that the prop won't be won't be displayed if it's included on the first line with the back ticks. Since there are 7 props that this applies to, maybe it would be better to have the full example in the docs instead of here.

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/plugins/pages.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
side_by_side: True
---
"""
import frontmatter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious to see how this library parsed YAML, given my exec / eval concerns when writing the code fence parser.

It looks like frontmatter depends on PyYAML's safe mode: https://github.com/eyeseast/python-frontmatter/blob/1c49958fdd504691fc842045425ea298316aa643/frontmatter/default_handlers.py#L224-L228

So then looking into PyYAML, I see some stuff like:

Note that the ability to construct an arbitrary Python object may be dangerous if you receive a YAML document from an untrusted source such as the Internet. The function yaml.safe_load limits this ability to simple Python objects like integers or lists.

from https://pyyaml.org/wiki/PyYAMLDocumentation

Then googled around yaml.safe_load and came across yaml/pyyaml#420

They fixed the issue which is great but the fact that an issue happened in the first place makes me feel a little concerned that there could be more issues lurking in that library.

So I might consider supporting a smaller subset of the YAML library and doing our own parsing with JSON, similar to what we did with the code fences.

I'll keep thinking a bit about this

Comment threaddash_labs/plugins/pages.py Outdated
<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to put this in the stylesheet that we document

Comment threaddash_labs/plugins/pages.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
if "app.layout" in code:
code = code.replace("app.layout", "layout")
if "layout" in code:
exec(code, scope)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling like we should lock this down a bit more and exclude folks from using exec within callbacks.
for users, this means:

  • MarkdownAIO can only be called outside of callbacks, meaning when the app starts up
  • All of the MarkdownAIO files would need to be written in advance of the app starting up

so this wouldn't be allowed:
pages/report.py

dash.register_page(__name__)
def layout():
return MarkdownAIO('report.md', exec_code=True)

But this would be allowed:
pages/report.py

dash.register_page(__name__)
layout = MarkdownAIO('report.md', exec_code=True)

I can't think of many examples where this would be insufficient and it would prevent users from seemingly innocent but very insecure code like:

template_report.md

# Report for {customer}
`` `python
dcc.Graph(figure=px.scatter(...))
`` `

app.py

app.layout=html.Div([
dcc.Dropdown(id='customer', options=['Acme', 'City']),
html.Div(id='content')
])
@callback(Output('content', 'children'), Input('customer', 'value'))defupdate(customer):
withopen('template_report.md', 'r') asf:
content=f.read()
customer_report=content.replace(customer=customer)
withopen('customer_report.md', 'w') asf:
f.write(customer_report)
returnMarkdownAIO('customer_report.md', exec_code=True)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to prevent exec from running in a callback would be to do:

def_exec(*args, **kwargs):
# flask raises an error if flask.request.path is called outside of a requesttry:
flask.request.pathexceptRuntimeErrorase:
raiseException('MarkdownAIO is being called with `exec_code=True` within a callback or flask request. This isn\'t supported. MarkdownAIO with exec can only be called when the app is starting')
returnexec(*args, **kwargs)

@AnnMarieWAnnMarieWFeb 15, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried this it prevented all exec s in the app. Checking for flask.has_request_context() worked though.

Will this be overly restrictive with a multi-page app using path variables and query strings? Also, many pages are functions even if there are no variables passed to the layout.

Update: This only works if it's called from a .py file. If the MarkdownAIO is inside a callback in a .md file, then
the flask.has_request_context() is False

new format for props in code blocks
use ast to remove app instance
removed dbc dependency
@AnnMarieWAnnMarieW mentioned this pull request Feb 10, 2022
12 tasks
@AnnMarieW

Copy link
Copy Markdown
CollaboratorAuthor

This was a cool project but did't make it across the finish line because there were concerns about executing the code securely.

There are some other community project that are good for project that are a mix of code and markdown text. See:

https://github.com/snehilvj/markdown2dash
https://github.com/snehilvj/dmc-docs
https://github.com/emilhe/dash-down

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AnnMarieW@chriddyp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

MarkdownAIO - #82

Closed
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO
Closed

MarkdownAIO#82
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO

Conversation

@AnnMarieW

@AnnMarieWAnnMarieW commented Jan 31, 2022

Copy link
Copy Markdown
Collaborator

Adds the new component MarkdownAIO

See the live demo

MarkdownAIO is a Dash feature that allows you to write Dash Apps as Markdown files. Simply pass in a Markdown file and MarkdownAIO will return a set of components with the option to display and/or execute code blocks. So it’s part:

  • Documentation helper
  • Markdown / Text authoring tool
  • Easy way to bring a Markdown file into an app without doing open('file.md')

It's also compatible with the pages/ api. The online docs for dash-labs is a multi-page app made with pages/ and each page is a Markdown file displayed using MarkdownAIO.


Here is a summary of the TODOs and questions:

  • To further reduce security risk with exec, check for local files only. Ensure that the file is within a certain directory, and by default that should be the parent directory of the main app but maybe we could create a way to override that. Similar to /assets?
    Update - require full path from pages/

  • css:

    • create a MarkdownAIO stylesheet?
    • eliminated dbc dependency (replace Rows and Cols with inline css)
    • document the default style in subcomponents. Or better yet - use a stylesheet?
      Status: inprogress
  • see todos in pages.py in _register_page_from_markdown_file()

  • [x ] use UUID for clipboard ID?

  • add ability to change defaults "globally" so it doesn't have to be done for each MarkdownAIO() instance.

  • add ability to embed another file within the Markdown file. Use case being, ability to keep the python app code in a separate file so you can run it individually. We could integrate jinja in here perhaps…: {% include code.py %}

  • Add Markdown files to hot reload in dash. That way users can have the same hot-reloading dev experience when working in markdown

  • if code block is not executed don't register callbacks and layout? Goal it to reduce the number of id clashes.

  • Need to remove if __name__ == "__main__": ... from the code blocks?

  • refactor _update_props() (It works, but it's kinda ugly)

  • rewrite the _remove_app_instance() to use the AST module

@AnnMarieW
AnnMarieW marked this pull request as ready for review January 31, 2022 17:21

@chriddypchriddyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking awesome! several small changes requested. once those are made, i'll take a deeper look at the docs 🙂

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated

- `exec_code` (boolean; default False):
If `True`, code blocks will be executed. This may also be set within the code block with the comment
# exec-code-true or # exec-code-false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe where this comment should exist. perhaps include a full example with the triple backticks

@AnnMarieWAnnMarieWFeb 2, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can actually be anywhere in the code block on a comment line. It's just that the prop won't be won't be displayed if it's included on the first line with the back ticks. Since there are 7 props that this applies to, maybe it would be better to have the full example in the docs instead of here.

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/plugins/pages.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
side_by_side: True
---
"""
import frontmatter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious to see how this library parsed YAML, given my exec / eval concerns when writing the code fence parser.

It looks like frontmatter depends on PyYAML's safe mode: https://github.com/eyeseast/python-frontmatter/blob/1c49958fdd504691fc842045425ea298316aa643/frontmatter/default_handlers.py#L224-L228

So then looking into PyYAML, I see some stuff like:

Note that the ability to construct an arbitrary Python object may be dangerous if you receive a YAML document from an untrusted source such as the Internet. The function yaml.safe_load limits this ability to simple Python objects like integers or lists.

from https://pyyaml.org/wiki/PyYAMLDocumentation

Then googled around yaml.safe_load and came across yaml/pyyaml#420

They fixed the issue which is great but the fact that an issue happened in the first place makes me feel a little concerned that there could be more issues lurking in that library.

So I might consider supporting a smaller subset of the YAML library and doing our own parsing with JSON, similar to what we did with the code fences.

I'll keep thinking a bit about this

Comment threaddash_labs/plugins/pages.py Outdated
<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to put this in the stylesheet that we document

Comment threaddash_labs/plugins/pages.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
if "app.layout" in code:
code = code.replace("app.layout", "layout")
if "layout" in code:
exec(code, scope)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling like we should lock this down a bit more and exclude folks from using exec within callbacks.
for users, this means:

  • MarkdownAIO can only be called outside of callbacks, meaning when the app starts up
  • All of the MarkdownAIO files would need to be written in advance of the app starting up

so this wouldn't be allowed:
pages/report.py

dash.register_page(__name__)
def layout():
return MarkdownAIO('report.md', exec_code=True)

But this would be allowed:
pages/report.py

dash.register_page(__name__)
layout = MarkdownAIO('report.md', exec_code=True)

I can't think of many examples where this would be insufficient and it would prevent users from seemingly innocent but very insecure code like:

template_report.md

# Report for {customer}
`` `python
dcc.Graph(figure=px.scatter(...))
`` `

app.py

app.layout=html.Div([
dcc.Dropdown(id='customer', options=['Acme', 'City']),
html.Div(id='content')
])
@callback(Output('content', 'children'), Input('customer', 'value'))defupdate(customer):
withopen('template_report.md', 'r') asf:
content=f.read()
customer_report=content.replace(customer=customer)
withopen('customer_report.md', 'w') asf:
f.write(customer_report)
returnMarkdownAIO('customer_report.md', exec_code=True)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to prevent exec from running in a callback would be to do:

def_exec(*args, **kwargs):
# flask raises an error if flask.request.path is called outside of a requesttry:
flask.request.pathexceptRuntimeErrorase:
raiseException('MarkdownAIO is being called with `exec_code=True` within a callback or flask request. This isn\'t supported. MarkdownAIO with exec can only be called when the app is starting')
returnexec(*args, **kwargs)

@AnnMarieWAnnMarieWFeb 15, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried this it prevented all exec s in the app. Checking for flask.has_request_context() worked though.

Will this be overly restrictive with a multi-page app using path variables and query strings? Also, many pages are functions even if there are no variables passed to the layout.

Update: This only works if it's called from a .py file. If the MarkdownAIO is inside a callback in a .md file, then
the flask.has_request_context() is False

new format for props in code blocks
use ast to remove app instance
removed dbc dependency
@AnnMarieWAnnMarieW mentioned this pull request Feb 10, 2022
12 tasks
@AnnMarieW

Copy link
Copy Markdown
CollaboratorAuthor

This was a cool project but did't make it across the finish line because there were concerns about executing the code securely.

There are some other community project that are good for project that are a mix of code and markdown text. See:

https://github.com/snehilvj/markdown2dash
https://github.com/snehilvj/dmc-docs
https://github.com/emilhe/dash-down

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AnnMarieW@chriddyp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

MarkdownAIO - #82

Closed
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO
Closed

MarkdownAIO#82
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO

Conversation

@AnnMarieW

@AnnMarieWAnnMarieW commented Jan 31, 2022

Copy link
Copy Markdown
Collaborator

Adds the new component MarkdownAIO

See the live demo

MarkdownAIO is a Dash feature that allows you to write Dash Apps as Markdown files. Simply pass in a Markdown file and MarkdownAIO will return a set of components with the option to display and/or execute code blocks. So it’s part:

  • Documentation helper
  • Markdown / Text authoring tool
  • Easy way to bring a Markdown file into an app without doing open('file.md')

It's also compatible with the pages/ api. The online docs for dash-labs is a multi-page app made with pages/ and each page is a Markdown file displayed using MarkdownAIO.


Here is a summary of the TODOs and questions:

  • To further reduce security risk with exec, check for local files only. Ensure that the file is within a certain directory, and by default that should be the parent directory of the main app but maybe we could create a way to override that. Similar to /assets?
    Update - require full path from pages/

  • css:

    • create a MarkdownAIO stylesheet?
    • eliminated dbc dependency (replace Rows and Cols with inline css)
    • document the default style in subcomponents. Or better yet - use a stylesheet?
      Status: inprogress
  • see todos in pages.py in _register_page_from_markdown_file()

  • [x ] use UUID for clipboard ID?

  • add ability to change defaults "globally" so it doesn't have to be done for each MarkdownAIO() instance.

  • add ability to embed another file within the Markdown file. Use case being, ability to keep the python app code in a separate file so you can run it individually. We could integrate jinja in here perhaps…: {% include code.py %}

  • Add Markdown files to hot reload in dash. That way users can have the same hot-reloading dev experience when working in markdown

  • if code block is not executed don't register callbacks and layout? Goal it to reduce the number of id clashes.

  • Need to remove if __name__ == "__main__": ... from the code blocks?

  • refactor _update_props() (It works, but it's kinda ugly)

  • rewrite the _remove_app_instance() to use the AST module

@AnnMarieW
AnnMarieW marked this pull request as ready for review January 31, 2022 17:21

@chriddypchriddyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking awesome! several small changes requested. once those are made, i'll take a deeper look at the docs 🙂

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated

- `exec_code` (boolean; default False):
If `True`, code blocks will be executed. This may also be set within the code block with the comment
# exec-code-true or # exec-code-false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe where this comment should exist. perhaps include a full example with the triple backticks

@AnnMarieWAnnMarieWFeb 2, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can actually be anywhere in the code block on a comment line. It's just that the prop won't be won't be displayed if it's included on the first line with the back ticks. Since there are 7 props that this applies to, maybe it would be better to have the full example in the docs instead of here.

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/plugins/pages.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
side_by_side: True
---
"""
import frontmatter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious to see how this library parsed YAML, given my exec / eval concerns when writing the code fence parser.

It looks like frontmatter depends on PyYAML's safe mode: https://github.com/eyeseast/python-frontmatter/blob/1c49958fdd504691fc842045425ea298316aa643/frontmatter/default_handlers.py#L224-L228

So then looking into PyYAML, I see some stuff like:

Note that the ability to construct an arbitrary Python object may be dangerous if you receive a YAML document from an untrusted source such as the Internet. The function yaml.safe_load limits this ability to simple Python objects like integers or lists.

from https://pyyaml.org/wiki/PyYAMLDocumentation

Then googled around yaml.safe_load and came across yaml/pyyaml#420

They fixed the issue which is great but the fact that an issue happened in the first place makes me feel a little concerned that there could be more issues lurking in that library.

So I might consider supporting a smaller subset of the YAML library and doing our own parsing with JSON, similar to what we did with the code fences.

I'll keep thinking a bit about this

Comment threaddash_labs/plugins/pages.py Outdated
<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to put this in the stylesheet that we document

Comment threaddash_labs/plugins/pages.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
if "app.layout" in code:
code = code.replace("app.layout", "layout")
if "layout" in code:
exec(code, scope)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling like we should lock this down a bit more and exclude folks from using exec within callbacks.
for users, this means:

  • MarkdownAIO can only be called outside of callbacks, meaning when the app starts up
  • All of the MarkdownAIO files would need to be written in advance of the app starting up

so this wouldn't be allowed:
pages/report.py

dash.register_page(__name__)
def layout():
return MarkdownAIO('report.md', exec_code=True)

But this would be allowed:
pages/report.py

dash.register_page(__name__)
layout = MarkdownAIO('report.md', exec_code=True)

I can't think of many examples where this would be insufficient and it would prevent users from seemingly innocent but very insecure code like:

template_report.md

# Report for {customer}
`` `python
dcc.Graph(figure=px.scatter(...))
`` `

app.py

app.layout=html.Div([
dcc.Dropdown(id='customer', options=['Acme', 'City']),
html.Div(id='content')
])
@callback(Output('content', 'children'), Input('customer', 'value'))defupdate(customer):
withopen('template_report.md', 'r') asf:
content=f.read()
customer_report=content.replace(customer=customer)
withopen('customer_report.md', 'w') asf:
f.write(customer_report)
returnMarkdownAIO('customer_report.md', exec_code=True)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to prevent exec from running in a callback would be to do:

def_exec(*args, **kwargs):
# flask raises an error if flask.request.path is called outside of a requesttry:
flask.request.pathexceptRuntimeErrorase:
raiseException('MarkdownAIO is being called with `exec_code=True` within a callback or flask request. This isn\'t supported. MarkdownAIO with exec can only be called when the app is starting')
returnexec(*args, **kwargs)

@AnnMarieWAnnMarieWFeb 15, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried this it prevented all exec s in the app. Checking for flask.has_request_context() worked though.

Will this be overly restrictive with a multi-page app using path variables and query strings? Also, many pages are functions even if there are no variables passed to the layout.

Update: This only works if it's called from a .py file. If the MarkdownAIO is inside a callback in a .md file, then
the flask.has_request_context() is False

new format for props in code blocks
use ast to remove app instance
removed dbc dependency
@AnnMarieWAnnMarieW mentioned this pull request Feb 10, 2022
12 tasks
@AnnMarieW

Copy link
Copy Markdown
CollaboratorAuthor

This was a cool project but did't make it across the finish line because there were concerns about executing the code securely.

There are some other community project that are good for project that are a mix of code and markdown text. See:

https://github.com/snehilvj/markdown2dash
https://github.com/snehilvj/dmc-docs
https://github.com/emilhe/dash-down

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AnnMarieW@chriddyp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

MarkdownAIO - #82

Closed
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO
Closed

MarkdownAIO#82
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO

Conversation

@AnnMarieW

@AnnMarieWAnnMarieW commented Jan 31, 2022

Copy link
Copy Markdown
Collaborator

Adds the new component MarkdownAIO

See the live demo

MarkdownAIO is a Dash feature that allows you to write Dash Apps as Markdown files. Simply pass in a Markdown file and MarkdownAIO will return a set of components with the option to display and/or execute code blocks. So it’s part:

  • Documentation helper
  • Markdown / Text authoring tool
  • Easy way to bring a Markdown file into an app without doing open('file.md')

It's also compatible with the pages/ api. The online docs for dash-labs is a multi-page app made with pages/ and each page is a Markdown file displayed using MarkdownAIO.


Here is a summary of the TODOs and questions:

  • To further reduce security risk with exec, check for local files only. Ensure that the file is within a certain directory, and by default that should be the parent directory of the main app but maybe we could create a way to override that. Similar to /assets?
    Update - require full path from pages/

  • css:

    • create a MarkdownAIO stylesheet?
    • eliminated dbc dependency (replace Rows and Cols with inline css)
    • document the default style in subcomponents. Or better yet - use a stylesheet?
      Status: inprogress
  • see todos in pages.py in _register_page_from_markdown_file()

  • [x ] use UUID for clipboard ID?

  • add ability to change defaults "globally" so it doesn't have to be done for each MarkdownAIO() instance.

  • add ability to embed another file within the Markdown file. Use case being, ability to keep the python app code in a separate file so you can run it individually. We could integrate jinja in here perhaps…: {% include code.py %}

  • Add Markdown files to hot reload in dash. That way users can have the same hot-reloading dev experience when working in markdown

  • if code block is not executed don't register callbacks and layout? Goal it to reduce the number of id clashes.

  • Need to remove if __name__ == "__main__": ... from the code blocks?

  • refactor _update_props() (It works, but it's kinda ugly)

  • rewrite the _remove_app_instance() to use the AST module

@AnnMarieW
AnnMarieW marked this pull request as ready for review January 31, 2022 17:21

@chriddypchriddyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking awesome! several small changes requested. once those are made, i'll take a deeper look at the docs 🙂

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated

- `exec_code` (boolean; default False):
If `True`, code blocks will be executed. This may also be set within the code block with the comment
# exec-code-true or # exec-code-false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe where this comment should exist. perhaps include a full example with the triple backticks

@AnnMarieWAnnMarieWFeb 2, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can actually be anywhere in the code block on a comment line. It's just that the prop won't be won't be displayed if it's included on the first line with the back ticks. Since there are 7 props that this applies to, maybe it would be better to have the full example in the docs instead of here.

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/plugins/pages.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
side_by_side: True
---
"""
import frontmatter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious to see how this library parsed YAML, given my exec / eval concerns when writing the code fence parser.

It looks like frontmatter depends on PyYAML's safe mode: https://github.com/eyeseast/python-frontmatter/blob/1c49958fdd504691fc842045425ea298316aa643/frontmatter/default_handlers.py#L224-L228

So then looking into PyYAML, I see some stuff like:

Note that the ability to construct an arbitrary Python object may be dangerous if you receive a YAML document from an untrusted source such as the Internet. The function yaml.safe_load limits this ability to simple Python objects like integers or lists.

from https://pyyaml.org/wiki/PyYAMLDocumentation

Then googled around yaml.safe_load and came across yaml/pyyaml#420

They fixed the issue which is great but the fact that an issue happened in the first place makes me feel a little concerned that there could be more issues lurking in that library.

So I might consider supporting a smaller subset of the YAML library and doing our own parsing with JSON, similar to what we did with the code fences.

I'll keep thinking a bit about this

Comment threaddash_labs/plugins/pages.py Outdated
<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to put this in the stylesheet that we document

Comment threaddash_labs/plugins/pages.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
if "app.layout" in code:
code = code.replace("app.layout", "layout")
if "layout" in code:
exec(code, scope)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling like we should lock this down a bit more and exclude folks from using exec within callbacks.
for users, this means:

  • MarkdownAIO can only be called outside of callbacks, meaning when the app starts up
  • All of the MarkdownAIO files would need to be written in advance of the app starting up

so this wouldn't be allowed:
pages/report.py

dash.register_page(__name__)
def layout():
return MarkdownAIO('report.md', exec_code=True)

But this would be allowed:
pages/report.py

dash.register_page(__name__)
layout = MarkdownAIO('report.md', exec_code=True)

I can't think of many examples where this would be insufficient and it would prevent users from seemingly innocent but very insecure code like:

template_report.md

# Report for {customer}
`` `python
dcc.Graph(figure=px.scatter(...))
`` `

app.py

app.layout=html.Div([
dcc.Dropdown(id='customer', options=['Acme', 'City']),
html.Div(id='content')
])
@callback(Output('content', 'children'), Input('customer', 'value'))defupdate(customer):
withopen('template_report.md', 'r') asf:
content=f.read()
customer_report=content.replace(customer=customer)
withopen('customer_report.md', 'w') asf:
f.write(customer_report)
returnMarkdownAIO('customer_report.md', exec_code=True)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to prevent exec from running in a callback would be to do:

def_exec(*args, **kwargs):
# flask raises an error if flask.request.path is called outside of a requesttry:
flask.request.pathexceptRuntimeErrorase:
raiseException('MarkdownAIO is being called with `exec_code=True` within a callback or flask request. This isn\'t supported. MarkdownAIO with exec can only be called when the app is starting')
returnexec(*args, **kwargs)

@AnnMarieWAnnMarieWFeb 15, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried this it prevented all exec s in the app. Checking for flask.has_request_context() worked though.

Will this be overly restrictive with a multi-page app using path variables and query strings? Also, many pages are functions even if there are no variables passed to the layout.

Update: This only works if it's called from a .py file. If the MarkdownAIO is inside a callback in a .md file, then
the flask.has_request_context() is False

new format for props in code blocks
use ast to remove app instance
removed dbc dependency
@AnnMarieWAnnMarieW mentioned this pull request Feb 10, 2022
12 tasks
@AnnMarieW

Copy link
Copy Markdown
CollaboratorAuthor

This was a cool project but did't make it across the finish line because there were concerns about executing the code securely.

There are some other community project that are good for project that are a mix of code and markdown text. See:

https://github.com/snehilvj/markdown2dash
https://github.com/snehilvj/dmc-docs
https://github.com/emilhe/dash-down

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AnnMarieW@chriddyp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

MarkdownAIO - #82

Closed
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO
Closed

MarkdownAIO#82
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO

Conversation

@AnnMarieW

@AnnMarieWAnnMarieW commented Jan 31, 2022

Copy link
Copy Markdown
Collaborator

Adds the new component MarkdownAIO

See the live demo

MarkdownAIO is a Dash feature that allows you to write Dash Apps as Markdown files. Simply pass in a Markdown file and MarkdownAIO will return a set of components with the option to display and/or execute code blocks. So it’s part:

  • Documentation helper
  • Markdown / Text authoring tool
  • Easy way to bring a Markdown file into an app without doing open('file.md')

It's also compatible with the pages/ api. The online docs for dash-labs is a multi-page app made with pages/ and each page is a Markdown file displayed using MarkdownAIO.


Here is a summary of the TODOs and questions:

  • To further reduce security risk with exec, check for local files only. Ensure that the file is within a certain directory, and by default that should be the parent directory of the main app but maybe we could create a way to override that. Similar to /assets?
    Update - require full path from pages/

  • css:

    • create a MarkdownAIO stylesheet?
    • eliminated dbc dependency (replace Rows and Cols with inline css)
    • document the default style in subcomponents. Or better yet - use a stylesheet?
      Status: inprogress
  • see todos in pages.py in _register_page_from_markdown_file()

  • [x ] use UUID for clipboard ID?

  • add ability to change defaults "globally" so it doesn't have to be done for each MarkdownAIO() instance.

  • add ability to embed another file within the Markdown file. Use case being, ability to keep the python app code in a separate file so you can run it individually. We could integrate jinja in here perhaps…: {% include code.py %}

  • Add Markdown files to hot reload in dash. That way users can have the same hot-reloading dev experience when working in markdown

  • if code block is not executed don't register callbacks and layout? Goal it to reduce the number of id clashes.

  • Need to remove if __name__ == "__main__": ... from the code blocks?

  • refactor _update_props() (It works, but it's kinda ugly)

  • rewrite the _remove_app_instance() to use the AST module

@AnnMarieW
AnnMarieW marked this pull request as ready for review January 31, 2022 17:21

@chriddypchriddyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking awesome! several small changes requested. once those are made, i'll take a deeper look at the docs 🙂

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated

- `exec_code` (boolean; default False):
If `True`, code blocks will be executed. This may also be set within the code block with the comment
# exec-code-true or # exec-code-false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe where this comment should exist. perhaps include a full example with the triple backticks

@AnnMarieWAnnMarieWFeb 2, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can actually be anywhere in the code block on a comment line. It's just that the prop won't be won't be displayed if it's included on the first line with the back ticks. Since there are 7 props that this applies to, maybe it would be better to have the full example in the docs instead of here.

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/plugins/pages.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
side_by_side: True
---
"""
import frontmatter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious to see how this library parsed YAML, given my exec / eval concerns when writing the code fence parser.

It looks like frontmatter depends on PyYAML's safe mode: https://github.com/eyeseast/python-frontmatter/blob/1c49958fdd504691fc842045425ea298316aa643/frontmatter/default_handlers.py#L224-L228

So then looking into PyYAML, I see some stuff like:

Note that the ability to construct an arbitrary Python object may be dangerous if you receive a YAML document from an untrusted source such as the Internet. The function yaml.safe_load limits this ability to simple Python objects like integers or lists.

from https://pyyaml.org/wiki/PyYAMLDocumentation

Then googled around yaml.safe_load and came across yaml/pyyaml#420

They fixed the issue which is great but the fact that an issue happened in the first place makes me feel a little concerned that there could be more issues lurking in that library.

So I might consider supporting a smaller subset of the YAML library and doing our own parsing with JSON, similar to what we did with the code fences.

I'll keep thinking a bit about this

Comment threaddash_labs/plugins/pages.py Outdated
<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to put this in the stylesheet that we document

Comment threaddash_labs/plugins/pages.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
if "app.layout" in code:
code = code.replace("app.layout", "layout")
if "layout" in code:
exec(code, scope)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling like we should lock this down a bit more and exclude folks from using exec within callbacks.
for users, this means:

  • MarkdownAIO can only be called outside of callbacks, meaning when the app starts up
  • All of the MarkdownAIO files would need to be written in advance of the app starting up

so this wouldn't be allowed:
pages/report.py

dash.register_page(__name__)
def layout():
return MarkdownAIO('report.md', exec_code=True)

But this would be allowed:
pages/report.py

dash.register_page(__name__)
layout = MarkdownAIO('report.md', exec_code=True)

I can't think of many examples where this would be insufficient and it would prevent users from seemingly innocent but very insecure code like:

template_report.md

# Report for {customer}
`` `python
dcc.Graph(figure=px.scatter(...))
`` `

app.py

app.layout=html.Div([
dcc.Dropdown(id='customer', options=['Acme', 'City']),
html.Div(id='content')
])
@callback(Output('content', 'children'), Input('customer', 'value'))defupdate(customer):
withopen('template_report.md', 'r') asf:
content=f.read()
customer_report=content.replace(customer=customer)
withopen('customer_report.md', 'w') asf:
f.write(customer_report)
returnMarkdownAIO('customer_report.md', exec_code=True)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to prevent exec from running in a callback would be to do:

def_exec(*args, **kwargs):
# flask raises an error if flask.request.path is called outside of a requesttry:
flask.request.pathexceptRuntimeErrorase:
raiseException('MarkdownAIO is being called with `exec_code=True` within a callback or flask request. This isn\'t supported. MarkdownAIO with exec can only be called when the app is starting')
returnexec(*args, **kwargs)

@AnnMarieWAnnMarieWFeb 15, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried this it prevented all exec s in the app. Checking for flask.has_request_context() worked though.

Will this be overly restrictive with a multi-page app using path variables and query strings? Also, many pages are functions even if there are no variables passed to the layout.

Update: This only works if it's called from a .py file. If the MarkdownAIO is inside a callback in a .md file, then
the flask.has_request_context() is False

new format for props in code blocks
use ast to remove app instance
removed dbc dependency
@AnnMarieWAnnMarieW mentioned this pull request Feb 10, 2022
12 tasks
@AnnMarieW

Copy link
Copy Markdown
CollaboratorAuthor

This was a cool project but did't make it across the finish line because there were concerns about executing the code securely.

There are some other community project that are good for project that are a mix of code and markdown text. See:

https://github.com/snehilvj/markdown2dash
https://github.com/snehilvj/dmc-docs
https://github.com/emilhe/dash-down

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AnnMarieW@chriddyp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

MarkdownAIO - #82

Closed
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO
Closed

MarkdownAIO#82
AnnMarieW wants to merge 41 commits into
plotly:mainfrom
AnnMarieW:MarkdownAIO

Conversation

@AnnMarieW

@AnnMarieWAnnMarieW commented Jan 31, 2022

Copy link
Copy Markdown
Collaborator

Adds the new component MarkdownAIO

See the live demo

MarkdownAIO is a Dash feature that allows you to write Dash Apps as Markdown files. Simply pass in a Markdown file and MarkdownAIO will return a set of components with the option to display and/or execute code blocks. So it’s part:

  • Documentation helper
  • Markdown / Text authoring tool
  • Easy way to bring a Markdown file into an app without doing open('file.md')

It's also compatible with the pages/ api. The online docs for dash-labs is a multi-page app made with pages/ and each page is a Markdown file displayed using MarkdownAIO.


Here is a summary of the TODOs and questions:

  • To further reduce security risk with exec, check for local files only. Ensure that the file is within a certain directory, and by default that should be the parent directory of the main app but maybe we could create a way to override that. Similar to /assets?
    Update - require full path from pages/

  • css:

    • create a MarkdownAIO stylesheet?
    • eliminated dbc dependency (replace Rows and Cols with inline css)
    • document the default style in subcomponents. Or better yet - use a stylesheet?
      Status: inprogress
  • see todos in pages.py in _register_page_from_markdown_file()

  • [x ] use UUID for clipboard ID?

  • add ability to change defaults "globally" so it doesn't have to be done for each MarkdownAIO() instance.

  • add ability to embed another file within the Markdown file. Use case being, ability to keep the python app code in a separate file so you can run it individually. We could integrate jinja in here perhaps…: {% include code.py %}

  • Add Markdown files to hot reload in dash. That way users can have the same hot-reloading dev experience when working in markdown

  • if code block is not executed don't register callbacks and layout? Goal it to reduce the number of id clashes.

  • Need to remove if __name__ == "__main__": ... from the code blocks?

  • refactor _update_props() (It works, but it's kinda ugly)

  • rewrite the _remove_app_instance() to use the AST module

@AnnMarieW
AnnMarieW marked this pull request as ready for review January 31, 2022 17:21

@chriddypchriddyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking awesome! several small changes requested. once those are made, i'll take a deeper look at the docs 🙂

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated

- `exec_code` (boolean; default False):
If `True`, code blocks will be executed. This may also be set within the code block with the comment
# exec-code-true or # exec-code-false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe where this comment should exist. perhaps include a full example with the triple backticks

@AnnMarieWAnnMarieWFeb 2, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can actually be anywhere in the code block on a comment line. It's just that the prop won't be won't be displayed if it's included on the first line with the back ticks. Since there are 7 props that this applies to, maybe it would be better to have the full example in the docs instead of here.

Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py
Comment threaddash_labs/plugins/pages.py Outdated
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
side_by_side: True
---
"""
import frontmatter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious to see how this library parsed YAML, given my exec / eval concerns when writing the code fence parser.

It looks like frontmatter depends on PyYAML's safe mode: https://github.com/eyeseast/python-frontmatter/blob/1c49958fdd504691fc842045425ea298316aa643/frontmatter/default_handlers.py#L224-L228

So then looking into PyYAML, I see some stuff like:

Note that the ability to construct an arbitrary Python object may be dangerous if you receive a YAML document from an untrusted source such as the Internet. The function yaml.safe_load limits this ability to simple Python objects like integers or lists.

from https://pyyaml.org/wiki/PyYAMLDocumentation

Then googled around yaml.safe_load and came across yaml/pyyaml#420

They fixed the issue which is great but the fact that an issue happened in the first place makes me feel a little concerned that there could be more issues lurking in that library.

So I might consider supporting a smaller subset of the YAML library and doing our own parsing with JSON, similar to what we did with the code fences.

I'll keep thinking a bit about this

Comment threaddash_labs/plugins/pages.py Outdated
<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to put this in the stylesheet that we document

Comment threaddash_labs/plugins/pages.py
Comment threaddash_labs/dashdown.py Outdated
Comment threaddash_labs/dashdown.py Outdated
if "app.layout" in code:
code = code.replace("app.layout", "layout")
if "layout" in code:
exec(code, scope)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling like we should lock this down a bit more and exclude folks from using exec within callbacks.
for users, this means:

  • MarkdownAIO can only be called outside of callbacks, meaning when the app starts up
  • All of the MarkdownAIO files would need to be written in advance of the app starting up

so this wouldn't be allowed:
pages/report.py

dash.register_page(__name__)
def layout():
return MarkdownAIO('report.md', exec_code=True)

But this would be allowed:
pages/report.py

dash.register_page(__name__)
layout = MarkdownAIO('report.md', exec_code=True)

I can't think of many examples where this would be insufficient and it would prevent users from seemingly innocent but very insecure code like:

template_report.md

# Report for {customer}
`` `python
dcc.Graph(figure=px.scatter(...))
`` `

app.py

app.layout=html.Div([
dcc.Dropdown(id='customer', options=['Acme', 'City']),
html.Div(id='content')
])
@callback(Output('content', 'children'), Input('customer', 'value'))defupdate(customer):
withopen('template_report.md', 'r') asf:
content=f.read()
customer_report=content.replace(customer=customer)
withopen('customer_report.md', 'w') asf:
f.write(customer_report)
returnMarkdownAIO('customer_report.md', exec_code=True)

@chriddypchriddypFeb 8, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to prevent exec from running in a callback would be to do:

def_exec(*args, **kwargs):
# flask raises an error if flask.request.path is called outside of a requesttry:
flask.request.pathexceptRuntimeErrorase:
raiseException('MarkdownAIO is being called with `exec_code=True` within a callback or flask request. This isn\'t supported. MarkdownAIO with exec can only be called when the app is starting')
returnexec(*args, **kwargs)

@AnnMarieWAnnMarieWFeb 15, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried this it prevented all exec s in the app. Checking for flask.has_request_context() worked though.

Will this be overly restrictive with a multi-page app using path variables and query strings? Also, many pages are functions even if there are no variables passed to the layout.

Update: This only works if it's called from a .py file. If the MarkdownAIO is inside a callback in a .md file, then
the flask.has_request_context() is False

new format for props in code blocks
use ast to remove app instance
removed dbc dependency
@AnnMarieWAnnMarieW mentioned this pull request Feb 10, 2022
12 tasks
@AnnMarieW

Copy link
Copy Markdown
CollaboratorAuthor

This was a cool project but did't make it across the finish line because there were concerns about executing the code securely.

There are some other community project that are good for project that are a mix of code and markdown text. See:

https://github.com/snehilvj/markdown2dash
https://github.com/snehilvj/dmc-docs
https://github.com/emilhe/dash-down

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@AnnMarieW@chriddyp