Conversation
- redcarpet gemを追加してMarkdownパースを実装 - カスタムボタンリンク記法を含むMarkdownHelperを実装 - マークダウンプレビュー用APIエンドポイントを作成 - 編集/プレビュータブ切り替え用のStimulusコントローラーを追加 - イベントフォームをマークダウンエディタコンポーネントに更新 - ヘルプページにマークダウン記法ガイドを追加 - マークダウンレンダリングの包括的なテストを追加 機能: - 標準マークダウン: 見出し、太字、斜体、リスト、リンク、テーブル、コード - カスタムボタンリンク記法: <button-link url="...">ラベル</button-link> - サニタイズによるXSS対策 - イベント作成・編集でのリアルタイムプレビュー 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、イベントの説明文と開催レポートの入力フィールドにMarkdown記法を導入し、コンテンツの表現力を向上させることを目的としています。標準的なMarkdown記法に加え、特定のURLを目立たせるためのカスタムボタンリンク記法も利用可能です。ユーザーは編集中の内容をリアルタイムで確認できるプレビュー機能を通じて、より直感的にコンテンツを作成できます。また、セキュリティ対策として厳格なサニタイズ処理が施されており、ヘルプページには新しい記法のガイドが追加されています。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
このプルリクエストは、イベント説明文と開催レポートにMarkdownサポートを追加するもので、素晴らしい機能改善ですね。redcarpet gemの導入、プレビュー機能、カスタムの<button-link>記法など、実装は全体的に堅牢です。特に、サーバーサイドでのプレビュー生成とサニタイズ、そしてそれに対応する単体テストとシステムテストが追加されている点は高く評価できます。
いくつかの改善点を提案しました。特に、セキュリティに関するgsubでのスクリプトタグ除去は、より堅牢なSafeListSanitizerに一本化することを強く推奨します。また、システムテストの安定性を向上させるための修正も提案しています。
これらの点を修正いただくことで、より安全で信頼性の高い機能になると思います。
app/helpers/markdown_helper.rb
Outdated
| # First remove script tags completely | ||
| html = html.gsub(/<script\b[^>]*>.*?<\/script>/im, '') |
There was a problem hiding this comment.
gsub を使った <script> タグの手動削除は、堅牢ではなく、バイパスされる可能性があります。例えば、<SCRipt> のような大文字小文字の違いや、onerror 属性を持つ他のタグは捕捉できません。90行目から使用している Rails::HTML5::SafeListSanitizer が、より安全で包括的なXSS対策を提供します。このサニタイザは、許可されていないタグ（<script> を含む）や属性をすべて削除するため、この gsub は不要であり、かえってセキュリティ対策の意図を曖昧にする可能性があります。サニタイザに一本化することをお勧めします。
spec/system/markdown_editor_spec.rb
Outdated
| # プレビューが表示される（非同期処理なので少し待つ） | ||
| sleep 1 | ||
|
|
||
| # 編集エリアが隠れている | ||
| expect(page).not_to have_field('イベント説明', visible: true) |
There was a problem hiding this comment.
sleep 1 の使用は、テストの実行環境によって挙動が変わり、テストが不安定になる（flaky test）原因となります。また、このテストではプレビューが正しく表示されたことの確認が抜けています。
Capybaraの待機機能を利用して、プレビュー内容がレンダリングされたことを確認することで、sleepを削除し、より堅牢なテストにすることができます。
# プレビューが表示され、編集エリアが隠れていることを確認
within(find('[data-markdown-editor-target="previewPane"]')) do
expect(page).to have_selector('strong', text: 'Bold text')
expect(page).to have_selector('em', text: 'italic text')
end
expect(page).not_to have_field('イベント説明', visible: true)- JavaScript: Prettierでフォーマット修正 - Ruby: RuboCopでスタイル違反を修正（文字列リテラル、Capybaマッチャー） - テスト: 曖昧なボタンマッチングとvisibility判定を修正 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Geminiのコードレビューフィードバックに基づく改善: - 冗長なscriptタグのgsub処理を削除 SafeListSanitizerが既に適切に処理するため不要 - sanitizationテストの期待値を修正 scriptタグの削除のみを検証するように変更 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
このプルリクエストは、イベント説明文と開催レポートにMarkdownサポートを追加するもので、素晴らしい機能追加です。redcarpet gemの導入、カスタムの<button-link>記法、リアルタイムプレビュー機能など、実装は全体的に良くできています。
いくつかの改善点を提案させていただきました。
MarkdownHelperの堅牢性を向上させるための正規表現とプレースホルダー生成方法の修正。MarkdownPreviewsControllerのロジックの簡素化。- システムテストの安定性を高めるための
sleepの削除。
これらの修正により、コードの品質と信頼性がさらに向上するかと思います。
| # カスタムボタンタグを抽出してプレースホルダーに置き換え | ||
| def extract_button_links(text) | ||
| buttons = [] | ||
| text_with_placeholders = text.gsub(/<button-link\s+url="([^"]+)">([^<]+)<\/button-link>/m) do |
There was a problem hiding this comment.
現在の正規表現 /<button-link\s+url="([^"]+)">([^<]+)<\/button-link>/m は、ボタンのラベルに < を含めることができず、堅牢性に欠けます。例えば、1 < 2 のようなテキストは使用できません。
より柔軟な非貪欲マッチ (.*?) を使用することをお勧めします。
/<button-link\s+url="([^"]+)">(.*?)<\/button-link>/mこれにより、ラベル内のテキストが link_to ヘルパーによって安全にエスケープされるようになります。
text_with_placeholders = text.gsub(/<button-link\s+url="([^"]+)">(.*?)<\/button-link>/m) do
app/helpers/markdown_helper.rb
Outdated
|
|
||
| buttons << { url: url, label: label } | ||
| # Use a placeholder that won't be affected by markdown parsing | ||
| "BUTTONPLACEHOLDER#{buttons.size - 1}ENDPLACEHOLDER" |
There was a problem hiding this comment.
"BUTTONPLACEHOLDER#{buttons.size - 1}ENDPLACEHOLDER" のような連番ベースのプレースホルダーは、ユーザーが同じ文字列を入力した場合に予期せぬ置換を引き起こす可能性があります。
SecureRandom.hex を使用して一意なプレースホルダーを生成する方がはるかに安全です。
placeholder = "BUTTONPLACEHOLDER-#{SecureRandom.hex(8)}"
buttons << { url: url, label: label, placeholder: placeholder }
placeholderこの変更を適用するには、restore_button_placeholders メソッドも更新して、インデックスではなく保存されたプレースホルダーを使用するように修正する必要があります。
| # プレビューが表示される（非同期処理なので少し待つ） | ||
| sleep 1 |
There was a problem hiding this comment.
システムテストで sleep を使用すると、テストが不安定になる（flaky test）可能性があります。特定の要素が表示されるのを待つなど、より堅牢な方法に修正することをお勧めします。この場合は、プレビューコンテンツがレンダリングされるのを待つことができます。
# プレビューが表示されるのを待つ
within(first('[data-markdown-editor-target="preview"]')) do
expect(page).to have_css('strong', text: 'Bold text')
expect(page).to have_css('em', text: 'italic text')
end| def create | ||
| text = params[:text] | ||
|
|
||
| if text.blank? | ||
| render json: { html: '' } | ||
| return | ||
| end | ||
|
|
||
| html = render_markdown(text) | ||
|
|
||
| render json: { html: html } | ||
| end |
There was a problem hiding this comment.
このメソッドは簡潔にできます。render_markdown ヘルパーは既に空のテキスト入力を処理して空文字列を返すため、コントローラーでの明示的なチェックは不要です。
def create
html = render_markdown(params[:text])
render json: { html: html }
end以下の改善を実施:
1. プレースホルダー生成をSecureRandomベースに変更
- 連番ベースからSecureRandom.hexに変更
- ユーザー入力との衝突を防ぐため、より安全な実装に
- restore_button_placeholdersメソッドも更新
2. MarkdownPreviewsControllerを簡潔化
- render_markdownが既に空文字列処理をしているため、
コントローラーでの冗長なblank?チェックを削除
- コードがより簡潔でメンテナンス性が向上
正規表現の変更については、セキュリティの観点から現状維持とした。
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
イベント説明と開催レポートにマークダウン機能を導入する素晴らしいプルリクエストですね。redcarpetの導入、プレビュー機能、カスタムの<button-link>記法など、機能が充実しており、ユーザーにとって非常に便利になると思います。特に、サーバーサイドでの厳格なサニタイズや、XSS対策を考慮した設計は高く評価できます。テストも単体テストとシステムテストの両方が追加されており、品質の高い変更だと感じました。1点、MarkdownHelperの実装について、より堅牢性を高めるための改善提案をコメントしましたので、ご確認いただければ幸いです。全体として、非常によくできた実装です。
| next '' unless url.match?(%r{\Ahttps?://}) | ||
|
|
||
| # 一意なプレースホルダーを生成（ユーザー入力との衝突を防ぐ） | ||
| placeholder = "BUTTONPLACEHOLDER-#{SecureRandom.hex(8)}" |
There was a problem hiding this comment.
現在のプレースホルダーの実装では、ユーザーが BUTTONPLACEHOLDER-xxxx のような文字列を意図的に、あるいは偶然入力した場合に、意図しないボタンに置換されてしまうわずかな可能性があります。
このリスクを軽減するために、プレースホルダーをHTMLコメントで囲むことをお勧めします。ユーザーがHTMLコメントを直接入力する可能性は低く、より安全になります。また、もし入力されたとしてもHTMLコメントはレンダリングされないため、問題が発生しにくいです。
placeholder = "<!-- BUTTONPLACEHOLDER-#{SecureRandom.hex(8)} -->"1 participant
概要
イベント説明文と開催レポートでマークダウン記法が使えるようになりました。
実装内容
1. Markdown対応
2. カスタムボタンリンク記法
チャットへのリンクなどを目立たせるため、独自のボタンリンク記法を追加：
青いボタンスタイルで表示され、
target="_blank"とrel="noopener noreferrer"が自動付与されます。3. プレビュー機能
4. ドキュメント
5. テスト
変更ファイル
Gemfile- redcarpet gem追加app/helpers/markdown_helper.rb- マークダウンレンダリングapp/controllers/markdown_previews_controller.rb- プレビューAPIapp/javascript/controllers/markdown_editor_controller.js- タブ切り替えapp/views/shared/_markdown_editor.html.erb- エディタコンポーネントapp/views/teams/events/_form.html.erb- フォーム更新app/views/teams/events/_main_column.html.erb- 表示更新app/views/home/help.html.erb- ヘルプページ更新config/routes.rb- プレビューAPIルート追加テスト計画
🤖 Generated with Claude Code