-
Notifications
You must be signed in to change notification settings - Fork 84
Documentation for Workflow Qureies with Markdown #267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Documentation for Workflow Qureies with Markdown #267
Conversation
| The `format` field should contain well-known MIME type identifiers: | ||
|
|
||
| - `text/markdown` - Markdown content | ||
| - `text/csv` - Comma-separated values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, right now we don't support any other options except text/markdown, so while it's not a bad idea to mention that these exist, we can't call these supported. they'll just not work
|
|
||
| ```json | ||
| { | ||
| "cadenceResponseType": "formattedData", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, this doesn't work currently
| } | ||
| ``` | ||
|
|
||
| ### Image Response |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't support images presently, it'd be nice to in the future, but we've not added it yet.
| } | ||
|
|
||
| // Helper function for CSV responses | ||
| func NewCSVQueryResponse(rows [][]string) PrerenderedQueryResponse { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not supported presently
|
|
||
| Taking input from a workflow and rendering it as HTML without sanitization is a potential XSS (Cross-Site Scripting) vector. An attacker could inject malicious HTML tags including: | ||
|
|
||
| - `<script>` tags for JavaScript execution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fairly sure this won't render with the markdown implementation, so while it's worth paying attention to generally, this specifically shouldn't be an issue. In general we want to get workflow owners to:
- Sanitize their inputs before rendering them in markdown
- Be aware that XSS remains a possibility and to be wary of this
But also to be aware that this feature uses react-markdown in the UI and tries its best to remove vulnerabilities should they be sent from the server-side. As a side-effect of that, more extensive variants of markdown including full html will not work.
So I think this section is good, we should remind workflow owners of their responsibility to security, but also it's worth pointing out that there are some protections in place and as a result only a more limited set of markdown will work.
|
|
||
| ```go | ||
| // Example sanitization for markdown content | ||
| func sanitizeMarkdown(input string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove an explicit implementation guideline since it'll vary by implementation.
| ### Supported Renderers | ||
|
|
||
| - **Markdown**: Rendered using a markdown parser with syntax highlighting | ||
| - **CSV**: Displayed as interactive tables with sorting/filtering |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only markdown is supported for now
| } | ||
| ``` | ||
|
|
||
| ### CLI Testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we need a CLI component? the query rendering is only supported in the web UI
|
@davidporter-id-au please let me know if anything is missing to add here. Addressed all comments, thanks for your time!! |
Whats new
Documentation for Workflow Qureies with Markdown examples;
Updated the pop-up to show this new documentation;
Tested
Locally