opencloud-eu
Paste external image sources into markdown editor
Description
Our current set of CSP rules doesn't allow pasting image sources as external URLs into a markdown file. This feels like a bug and we should improve it. At the same time we don't want to open an attack vector for XSS attacks, tracking etc via malicious images.
User Stories
-
As a user writing markdown files, I want to paste external image urls and have a rendered preview so that I don't need to bloat the file size of my markdown file.
-
As an admin I want to protect my users from XSS attacks and tracking via external image urls so that I sleep well.
Value
External images in markdown files.
Acceptance Criteria
- Pasting a URL to an external image source leads to the image being rendered in the web ui
- Downloading the markdown file doesn't break the image rendering in a locally installed editor/viewer
- Uploading a markdown file with external image sources leads to the images being rendered in the web ui
- The thumbnailer accepts external image URLs as input
- The image is served via an internal (own) URL
- The output is XSS-safe, i.e. sanitized and free of malicious code
note: a CSP rule of img-src: * makes all of this possible but is considered dangerous. Hence we need another solution.
Definition of ready
- [ ] Everybody needs to understand the value written in the user story
- [ ] Acceptance criteria have to be defined
- [ ] All dependencies of the user story need to be identified
- [ ] Feature should be seen from an end user perspective
- [ ] Story has to be estimated
- [ ] Story points need to be less than 20
Definition of done
- Functional requirements
- [ ] Functionality described in the user story works
- [ ] Acceptance criteria are fulfilled
- Quality
- [ ] Code review happened
- [ ] CI is green (that includes new and existing automated tests)
- [ ] Critical code received unit tests by the developer
- Non-functional requirements
- [ ] No sonar cloud issues
- Configuration changes
- [ ] The next branch of the OpenCloud charts is compatible
See https://github.com/opencloud-eu/opencloud/pull/1101 for discussions revolving around the topic.
We could use our thumbnailing service to recreate images from external sites. That moves the attack vector to exploiting the thumbnail service, or rather the library we use, but IMO we should treat the thumbnailing service like antivirus or tika. They should run in a locked down docker container without internet access and we send them the bytes.
We could convert images to base64 blobs and embed them?
Which already works in our editor:
@micbar this is kind of ugly and not user friendly. No one wants to have their markdown file have thousands of lines with base64 glibberish
@micbar this is kind of ugly and not user friendly. No one wants to have their markdown file have thousands of lines with base64 glibberish
I see. But that fulfills the requirement that the file can be downloaded and continues to work offline.
Big no. Please let's try to implement software that not "somehow just works" but make it high quality.
I think the idea of GitHub is fantastic. They leave the URL's in the markdown as they are but replace them during the render process. Out thumbnailer could fulfill this, but needs to be able to read images from the www
How would you fulfil the requirement:
downloading the markdown file doesn't break the image rendering in a locally installed editor/viewer
How would you fulfil the requirement:
downloading the markdown file doesn't break the image rendering in a locally installed editor/viewer
Just means that we must not replace the image urls with our own urls in the original markdown content. The idea that @AlexAndBear mentions fulfills this.
This is my web POC: https://github.com/opencloud-eu/web/pull/863
it proofs, that we are able to render images from different sources (e.G) the thumbnailer while leaving the original markdown-content intact
Here's some info about what github is doing https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/about-anonymized-urls
For proxying the image downloads they're apparently using https://github.com/atmos/camo (archived since 2021 🤷♂)
Ok, replacing the markdown with a request to the opencloud domain removes the necessity for a CSP rule. But it kind of defeats the purpose of having CSP rules to prevent potentially malicious external content from reaching the users browser. If we do this we absolutely have to pass the image through the thumbnailer. We can cache the image and make a conditional get request to the external source to prevent downloading and recreating the thumbnail again.
We must not implement a generic proxy that can felch any external URL to embed it.
I would prefer if we automatically save a dragged image to the space and allow embedding images that way.
That being said, I can imagine use cases that require embedding status icons from external systems.
I think for that we need to change our thumbnailer implementation quite significantly, because it currently can only create thumbnails from images that it fetches via CS3/Webdav. Related: https://github.com/opencloud-eu/opencloud/issues/1128
Ok, replacing the markdown with a request to the opencloud domain removes the necessity for a CSP rule. But it kind of defeats the purpose of having CSP rules to prevent potentially malicious external content from reaching the users browser.
Right.
If we do this we absolutely have to pass the image through the thumbnailer.
Even with that, I don't think we can usefully prevent malicious content from reaching the browser. Unless we blacklist all the "interesting" formats (like e.g. SVG). Or are you imaging having the (somehow sandboxed) thumbnailer convert SVG so some less problematic format (e.g. jpeg)? I don't think that'll be of much use.
We can cache the imageand make a conditional get request to the external source to prevent downloading and recreating the thumbnail again.
As far as I understand that is basically what camo is doing.
I would prefer if we atomatically save a dragged image to the space and allow embedding images that way.
Agreed.
I would prefer if we atomatically save a dragged image to the space and allow embedding images that way.
So you want to vendor-lock markdown files into OpenCloud? Because the markdown file wouldn't work anymore if I download it. That's not a useful approach. Also, if images change externally (see e.g. status images in github repo readmes) I will never get an update.
Research Ticket: https://github.com/opencloud-eu/opencloud/issues/1145
Related issue with CSP Rules: https://github.com/opencloud-eu/opencloud/issues/1388
