Skip to content
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

feat: mermaid.js 対応 #150

Merged
merged 15 commits into from
Jun 7, 2021
Merged

Conversation

cm-wada-yusuke
Copy link
Member

@cm-wada-yusuke cm-wada-yusuke commented May 30, 2021

resolve: zenn-dev/zenn-community#299

SVG が生成されることから、変換負荷やキャッシュ量を考慮します。markdownToHtml では一次変換し、embed で埋め込み表示する方針です。

  • 方針の確認
  • idの生成方法について確認
  • サイズの確認
  • パースエラーへの対処
  • テストの充実

以下のようなイメージ

image

なぜやりたいか

Zenn における記事の新しい埋め込み・レンダリング対応は、対象ライブラリやツールが以下をすべて満たす場合に対応するという方針を考えています。

  • 十分な利用実績があること
  • セキュリティリスクが発生したときに、対応した実績があること
  • 現在の仕組みでは代わりがききづらいこと(手元で一度画像に変換して画像として貼り付けるしかない、など)
  • 記事の質を向上させるために寄与してくれること
  • Zenn の開発ポリシーの沿っていること

いくつか開発チームの主観的な指標もありますが、定量評価のみだとハックされる恐れがあるためです。mermaid.jsは、記事にグラフやシーケンス図の図示を可能とし、ITシステムにおける上流工程・設計工程の知見や、機器同士のシーケンスなどを表現することを助けてくれるはずです。

レンダリング方針

当初より大きくは変えておらず、markdownToHtml では <embed-mermaid> へ変換し、 zenn-embed-elements で CDN から mermaid.js をブラウザ側で読み込んだ上で、mermaidAPI という ブラウザ側でレンダリングできる 関数 を利用しています。これによりsvgコードによるキャッシュの肥大化を防ぎつつも、読み込んだ瞬間に勝手にレンダリングはさせずこちらのバリデーションを通した上でレンダリングすることで、セキュリティリスクを最小限に押さえています。

セキュリティリスクへの考え方

利用するバージョンを現時点で最新の 8.10.x に固定します。

XSS

過去、XSSが存在し、mermaid.js側で対応した実績がありました。

8.2.3で修正されました。

基本的には Markdown をレンダリングする際に escapeHtml し、エスケープした文字列を mermaidAPI.render() へ渡すことになります。なので最後にはセキュリティ対策はライブラリ側に委ねられており、私達としてはリスクの早期発見と無効化を行う必要があるという認識です。

Interaction

mermaid は interactiion 機能を提供しており、クリックイベントに対して図示を変えるといった表現力があります。しかしクリックイベントはセキュリティリスクをはらんでいると考えており、以下の措置をとっています。

  • レンダリング設定の securityLebel を最高の strict とする(GitLabも同様)。このレベルはクリックイベントが無効化される
  • レンダリング時にレンダリング済み関数のコールバックでバインドされたイベントが取得できるが、これをZenn側で破棄する

「Interactionは使えないよ」ということはユーザーマニュアルにも記載するつもりです。

相談事項

レビューいただいたいのですが(お忙しいところすいません)、中でもバリデーションエラー発生時の表示をご確認いただきたく。現状は図の代わりに <ul><li> でこんな感じの表示にしています。

image

@cm-wada-yusuke
Copy link
Member Author

@catnose99 お時間あるときに以下2点、見解を教えていただけると嬉しいです

  1. markdownToHtml では <embed-> し、 zenn-embed-elements で CDN から mermaid.js をブラウザ側で読み込む方針です。まずこの方針が認識一致しているかどうか
  2. zenn-embed-elements で mermaid の div に id を割り当てる必要があります。コンテンツからmd5ハッシュを生成してそれをIDにしようと考えているのですが、zenn-embed-elements にライブラリをインストールするのはあまり好ましくないでしょうか。zenn-markdown-html 側で何かしら 生成してそれをIDとする方法もあるかなと考えています

@cm-wada-yusuke
Copy link
Member Author

cm-wada-yusuke commented May 31, 2021

TODO:
セキュリティリスクに対してGitLab がどのような実装をとっているか解釈し、Zennでのリスクを評価する。

参考:
GitLab の実装
https://github.com/gitlabhq/gitlabhq/blob/42d13aebd3c47671337d871e8b349385dade5252/app/assets/javascripts/behaviors/markdown/render_mermaid.js

@cm-wada-yusuke cm-wada-yusuke added the feature 機能追加・改善 label Jun 5, 2021
@cm-wada-yusuke cm-wada-yusuke changed the title [WIP] mermaid.js 対応 feat: mermaid.js 対応 Jun 5, 2021
@@ -23,3 +23,192 @@ https://gist.github.com/mattpodwysocki/218388
@[gist](https://gist.github.com/hofmannsven/9164408?file=my.cnf)

ssfafafaffafa

Copy link
Member Author

Choose a reason for hiding this comment

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

[note] サンプルを記載していますがこんなに数がいらなければ言ってください。

N-->O
O-->P
P-->ID1[ノード1<br>ノード2]
```
Copy link
Member Author

@cm-wada-yusuke cm-wada-yusuke Jun 5, 2021

Choose a reason for hiding this comment

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

[note]

  • グラフかつチェイン(&)を使った例
  • 横幅を越えようとすると図と文字が小さくなる例

image

one --> two
three --> two
two --> c2
```
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] フローチャートの例

光輝-->>アリス: Great!
光輝->>Bob: How about you?
Bob-->>光輝: Jolly good!
```
Copy link
Member Author

Choose a reason for hiding this comment

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

[note]

  • シーケンス図の例
  • 日本語の例

image

+bool is_wild
+run()
}
```
Copy link
Member Author

Choose a reason for hiding this comment

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

[note]

  • クラス図の例

ScrollLockOn --> ScrollLockOff : EvScrollLockPressed

}
```
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] 状態遷移図の例

A-->B & C-->D & E-->F & Z-->X;
A-->B & C-->D & E-->F & Z-->X;
A-->B & C-->D & E-->F & Z-->X;
```
Copy link
Member Author

@cm-wada-yusuke cm-wada-yusuke Jun 5, 2021

Choose a reason for hiding this comment

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

[note]

  • エラーの例
  • 表示を相談したい

image

click alert`md5_salt` eval "Tooltip for a callback"
click B "javascript:alert('XSS')" "This is a tooltip for a link"
link Zebra "http://www.github.com" "This is a link"
```
Copy link
Member Author

Choose a reason for hiding this comment

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

[note]

  • mermaid 側のセキュリティ対策により link Zebra "http://www.github.com" "This is a link" は文法エラーになる例

alert`md5_salt`-->B;
click alert`md5_salt` eval "Tooltip for a callback"
click B "javascript:alert('XSS')" "This is a tooltip for a link"
```
Copy link
Member Author

Choose a reason for hiding this comment

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

[note]

  • prevent xss な例
  • 基本的には escapeHTML 済みのコードを mermaidAPI がレンダリングすることになるのでXSSは最後は mermaid 側にゆだねられます

yes: (source.match(/&/g) || []).length > MAX_CHAINING_OF_LINKS_LIMIT,
message: `<li>ブロックあたりの&によるチェイン上限は${MAX_CHAINING_OF_LINKS_LIMIT}です</li>`,
},
};
Copy link
Member Author

@cm-wada-yusuke cm-wada-yusuke Jun 5, 2021

Choose a reason for hiding this comment

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

[note] エラー時の表示方法を相談したいです。変わりのHTMLを表示するのは GitLab のマネです。

@@ -2,7 +2,7 @@ module.exports = {
globals: {
'ts-jest': {
// avoid "jsx" treated as "preserved"
tsConfig: 'tsconfig.json',
tsconfig: 'tsconfig.json',
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] warning がコンソールにでてたのでついでに

}
return defaultRender(tokens, idx, options, env, slf);
};
}
Copy link
Member Author

@cm-wada-yusuke cm-wada-yusuke Jun 5, 2021

Choose a reason for hiding this comment

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

[note] fence ルールへ追加することになります。<embed-mermaid> を embed ライブラリが探し当てます。preタグにclass="mermaid"としてしまうと mermaid.js を読み込んだ瞬間にレンダリングが走ってしまうのでそれぞ防ぐためにzenn-mermaidとしています。

@cm-wada-yusuke
Copy link
Member Author

@catnose99 もろもろ対応しまして、月曜日以降レビューいただければと思います:bow:

@cm-wada-yusuke cm-wada-yusuke marked this pull request as ready for review June 5, 2021 15:31
@cm-wada-yusuke cm-wada-yusuke changed the base branch from main to canary June 7, 2021 01:22
click B "javascript:alert('XSS')" "This is a tooltip for a link"
link Zebra "http://www.github.com" "This is a link"
```
Copy link
Member Author

Choose a reason for hiding this comment

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

[note]

  • サンプルを書く先のファイル(embed.md or example.md)
  • どのくらい例を書くか

アドバイスいただければと:bow:

mermaid!.mermaidAPI.parse(source);
return true;
} catch (e) {
return false;
Copy link
Contributor

@catnose99 catnose99 Jun 7, 2021

Choose a reason for hiding this comment

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

「レンダリングがなぜか上手くいかない」というケースでの原因究明のため、eを一応consoleに出力するようにしますか?

Copy link
Member Author

Choose a reason for hiding this comment

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

こちら承知しました。

@cm-wada-yusuke
Copy link
Member Author

@catnose99 出力するよう修正しました。
console.error だとプレビューで保存するたびに赤くなってちょっと体験が微妙だったので console.log にしています。

@catnose99 catnose99 self-requested a review June 7, 2021 07:18
Copy link
Contributor

@catnose99 catnose99 left a comment

Choose a reason for hiding this comment

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

いい感じです!ありがとうございます!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 機能追加・改善
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mermaid.js で記述された UML 図のレンダリング
2 participants