From ead1e8f94c83beeab5a7dc6cf81208b4be13681c Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 1 Nov 2026 22:12:17 -1000 Subject: [PATCH 1/2] Sanitize comment markdown on the ReScript page The ReScript comment view rendered marked output without sanitizing it, unlike the React and server component views. Pass it through sanitize-html with the same defaults the React view uses, and cover it with a system spec. Co-Authored-By: Claude Opus 5.5 --- .../rescript/CommentList/Comment/Comment.res | 3 ++- .../comments/rescript/bindings/SanitizeHtml.res | 1 + spec/rescript/rescript_spec.rb | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 client/app/bundles/comments/rescript/bindings/SanitizeHtml.res diff --git a/client/app/bundles/comments/rescript/CommentList/Comment/Comment.res b/client/app/bundles/comments/rescript/CommentList/Comment/Comment.res index 817fe6b16..c1f3aa06f 100644 --- a/client/app/bundles/comments/rescript/CommentList/Comment/Comment.res +++ b/client/app/bundles/comments/rescript/CommentList/Comment/Comment.res @@ -1,7 +1,8 @@ @react.component let make = (~comment: Actions.Fetch.t, ~cssTransitionGroupClassNames) => { let rawMarkup = Marked.marked(comment.text, {gfm: true}) - let innerHTML = {"__html": rawMarkup} + let sanitizedRawMarkup = SanitizeHtml.sanitize(rawMarkup) + let innerHTML = {"__html": sanitizedRawMarkup} let nodeRef = React.useRef(Js.Nullable.null) // The 500 must correspond to the 0.5s in: diff --git a/client/app/bundles/comments/rescript/bindings/SanitizeHtml.res b/client/app/bundles/comments/rescript/bindings/SanitizeHtml.res new file mode 100644 index 000000000..1be73df1d --- /dev/null +++ b/client/app/bundles/comments/rescript/bindings/SanitizeHtml.res @@ -0,0 +1 @@ +@module("sanitize-html") external sanitize: string => string = "default" diff --git a/spec/rescript/rescript_spec.rb b/spec/rescript/rescript_spec.rb index 2d01444d4..56815dfb3 100644 --- a/spec/rescript/rescript_spec.rb +++ b/spec/rescript/rescript_spec.rb @@ -29,6 +29,23 @@ end end + describe "comment markdown rendering" do + before do + Comment.create!( + author: "Author", + text: '**bold** ' + ) + visit "/rescript" + end + + it "keeps markdown formatting and removes unsafe HTML" do + expect(page).to have_css(".js-comment-text strong", text: "bold") + expect(page).to have_no_css(".js-comment-text img") + expect(page).to have_no_css(".js-comment-text script") + expect(page.evaluate_script("window.xssRan")).to be_nil + end + end + describe "form submission functions" do let(:comment) { Comment.new(author: "Author", text: "This is a comment") } let(:author_field) { "comment_author" } From 11c58d20900547aa8a0dc4005b8bb0d530209947 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 1 Nov 2026 22:17:14 -1000 Subject: [PATCH 2/2] Count hidden elements in the ReScript sanitize spec Capybara skips hidden elements by default, so the script check could never fail, and the handler check depended on timing. Check the DOM directly. Co-Authored-By: Claude Opus 5.5 --- spec/rescript/rescript_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/rescript/rescript_spec.rb b/spec/rescript/rescript_spec.rb index 56815dfb3..fbde6caf7 100644 --- a/spec/rescript/rescript_spec.rb +++ b/spec/rescript/rescript_spec.rb @@ -40,9 +40,8 @@ it "keeps markdown formatting and removes unsafe HTML" do expect(page).to have_css(".js-comment-text strong", text: "bold") - expect(page).to have_no_css(".js-comment-text img") - expect(page).to have_no_css(".js-comment-text script") - expect(page.evaluate_script("window.xssRan")).to be_nil + expect(page).to have_no_css(".js-comment-text img", visible: :all) + expect(page).to have_no_css(".js-comment-text script", visible: :all) end end