From ab0fb81479ce16f313419c0b07635e3de29273f9 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Fri, 20 Oct 2023 09:32:16 -0400
Subject: [PATCH] Configure brakeman to ignore url safe preview card urls
 (#25883)

---
 app/helpers/formatting_helper.rb              |  4 ++
 .../trends/links/_preview_card.html.haml      |  2 +-
 config/brakeman.ignore                        | 39 -------------------
 config/brakeman.yml                           |  2 +
 .../links/_preview_card.html.haml_spec.rb     | 20 ++++++++++
 5 files changed, 27 insertions(+), 40 deletions(-)
 delete mode 100644 config/brakeman.ignore
 create mode 100644 spec/views/admin/trends/links/_preview_card.html.haml_spec.rb

diff --git a/app/helpers/formatting_helper.rb b/app/helpers/formatting_helper.rb
index 5ab0c8c7ba..7d1423e52d 100644
--- a/app/helpers/formatting_helper.rb
+++ b/app/helpers/formatting_helper.rb
@@ -9,6 +9,10 @@ module FormattingHelper
     TextFormatter.new(text, options).to_s
   end
 
+  def url_for_preview_card(preview_card)
+    preview_card.url
+  end
+
   def extract_status_plain_text(status)
     PlainTextFormatter.new(status.text, status.local?).to_s
   end
diff --git a/app/views/admin/trends/links/_preview_card.html.haml b/app/views/admin/trends/links/_preview_card.html.haml
index 1ca3483715..ee3774790c 100644
--- a/app/views/admin/trends/links/_preview_card.html.haml
+++ b/app/views/admin/trends/links/_preview_card.html.haml
@@ -4,7 +4,7 @@
 
   .batch-table__row__content.pending-account
     .pending-account__header
-      = link_to preview_card.title, preview_card.url
+      = link_to preview_card.title, url_for_preview_card(preview_card)
 
       %br/
 
diff --git a/config/brakeman.ignore b/config/brakeman.ignore
deleted file mode 100644
index d5c0b94436..0000000000
--- a/config/brakeman.ignore
+++ /dev/null
@@ -1,39 +0,0 @@
-{
-  "ignored_warnings": [
-    {
-      "warning_type": "Cross-Site Scripting",
-      "warning_code": 4,
-      "fingerprint": "cd5cfd7f40037fbfa753e494d7129df16e358bfc43ef0da3febafbf4ee1ed3ac",
-      "check_name": "LinkToHref",
-      "message": "Potentially unsafe model attribute in `link_to` href",
-      "file": "app/views/admin/trends/links/_preview_card.html.haml",
-      "line": 7,
-      "link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
-      "code": "link_to((Unresolved Model).new.title, (Unresolved Model).new.url)",
-      "render_path": [
-        {
-          "type": "template",
-          "name": "admin/trends/links/index",
-          "line": 49,
-          "file": "app/views/admin/trends/links/index.html.haml",
-          "rendered": {
-            "name": "admin/trends/links/_preview_card",
-            "file": "app/views/admin/trends/links/_preview_card.html.haml"
-          }
-        }
-      ],
-      "location": {
-        "type": "template",
-        "template": "admin/trends/links/_preview_card"
-      },
-      "user_input": "(Unresolved Model).new.url",
-      "confidence": "Weak",
-      "cwe_id": [
-        79
-      ],
-      "note": ""
-    }
-  ],
-  "updated": "2023-07-12 11:20:51 -0400",
-  "brakeman_version": "6.0.0"
-}
diff --git a/config/brakeman.yml b/config/brakeman.yml
index 95ebc13865..9ac95c8006 100644
--- a/config/brakeman.yml
+++ b/config/brakeman.yml
@@ -1,3 +1,5 @@
 ---
 :skip_checks:
   - CheckPermitAttributes
+:url_safe_methods:
+  - url_for_preview_card
diff --git a/spec/views/admin/trends/links/_preview_card.html.haml_spec.rb b/spec/views/admin/trends/links/_preview_card.html.haml_spec.rb
new file mode 100644
index 0000000000..82a1dee6d7
--- /dev/null
+++ b/spec/views/admin/trends/links/_preview_card.html.haml_spec.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe 'admin/trends/links/_preview_card.html.haml' do
+  it 'correctly escapes user supplied url values' do
+    form = instance_double(ActionView::Helpers::FormHelper, check_box: nil)
+    trend = PreviewCardTrend.new(allowed: false)
+    preview_card = Fabricate.build(
+      :preview_card,
+      url: 'https://host.example/path?query=<script>',
+      trend: trend,
+      title: 'Fun'
+    )
+
+    render partial: 'admin/trends/links/preview_card', locals: { preview_card: preview_card, f: form }
+
+    expect(rendered).to include('<a href="https://host.example/path?query=&lt;script&gt;">Fun</a>')
+  end
+end