Skip to content

Commit

Permalink
Configure brakeman to ignore url safe preview card urls
Browse files Browse the repository at this point in the history
These values are validated on the way to being saved, and html escaped
by Rails in the views.

The inclusion of the helper method here is a bit of a workaround to give
brakeman a method name to ignore.
  • Loading branch information
mjankowski committed Oct 19, 2023
1 parent 9f218c9 commit 7f0dbf9
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 40 deletions.
4 changes: 4 additions & 0 deletions app/helpers/formatting_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ def linkify(text, options = {})
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
Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/trends/links/_preview_card.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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/

Expand Down
39 changes: 0 additions & 39 deletions config/brakeman.ignore

This file was deleted.

2 changes: 2 additions & 0 deletions config/brakeman.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
---
:skip_checks:
- CheckPermitAttributes
:url_safe_methods:
- url_for_preview_card
20 changes: 20 additions & 0 deletions spec/views/admin/trends/links/_preview_card.html.haml_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 7f0dbf9

Please sign in to comment.