From e6f131079240d23df56e325ea91752587a6f86e8 Mon Sep 17 00:00:00 2001 From: Philippe Date: Wed, 1 Jul 2026 14:09:22 +0200 Subject: [PATCH 1/3] feat: auto rel=noopener on target=_blank links (configurable) --- CHANGELOG.md | 7 +++++ lib/activemail/components/base.rb | 14 +++++++-- lib/activemail/components/button.rb | 4 +-- lib/activemail/components/cta.rb | 2 +- lib/activemail/components/menu_item.rb | 2 +- lib/activemail/configuration.rb | 40 ++++++++++++++------------ lib/activemail/version.rb | 2 +- test/components/button_test.rb | 25 ++++++++++++++++ test/components/cta_test.rb | 20 +++++++++++++ test/components/menu_test.rb | 13 +++++++++ test/configuration_test.rb | 19 ++++++++++++ 11 files changed, 122 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c18af5e..04aad56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.2.0] - 2026-07-01 + +### Added + +- `config.blank_link_rel` (default `"noopener"`): a `rel` is now emitted automatically on + `target="_blank"` anchors. Set `nil` to disable; an explicit `rel="…"` still wins. + ## [1.1.1] - 2026-06-16 ### Added diff --git a/lib/activemail/components/base.rb b/lib/activemail/components/base.rb index 77e5a0b..3ec6054 100644 --- a/lib/activemail/components/base.rb +++ b/lib/activemail/components/base.rb @@ -28,8 +28,9 @@ class Base abstract! + # 'rel' is emitted by #link_attributes; passing it through too would duplicate it. IGNORED_ON_PASSTHROUGH = T.let( - %w[class id href size large no-expander small target up size-sm size-lg style].freeze, + %w[class id href size large no-expander small target rel up size-sm size-lg style].freeze, T::Array[String] ) @@ -102,9 +103,16 @@ def combine_attributes(node, extra_classes = nil) [pass_through_attributes(node), %(class="#{escape_attr(classes)}")].join end + # An explicit rel wins over the injected blank_link_rel default. sig { params(node: Nokogiri::XML::Node).returns(String) } - def target_attribute(node) - node.attributes['target'] ? %( target="#{escape_attr(node.attributes['target'])}") : '' + def link_attributes(node) + target = node.attributes['target']&.value + rel = node.attributes['rel']&.value + rel ||= ActiveMail.configuration.blank_link_rel if target == '_blank' + [ + target ? %( target="#{escape_attr(target)}") : '', + rel ? %( rel="#{escape_attr(rel)}") : '' + ].join end # Outlook-safe nested-table structure kept in one place for ') + + assert_includes output, 'rel="noopener"' + end + + def test_non_blank_target_gets_no_rel + output = render('') + + refute_includes output, 'rel=' + end + + def test_no_target_gets_no_rel + output = render('') + + refute_includes output, 'rel=' + end + + def test_explicit_rel_wins_without_duplication + output = render('') + + assert_includes output, 'rel="noopener noreferrer"' + assert_equal 1, output.scan('rel="').size + end + def test_classes_are_merged_with_button output = render('') diff --git a/test/components/cta_test.rb b/test/components/cta_test.rb index cc7c078..0f5e110 100644 --- a/test/components/cta_test.rb +++ b/test/components/cta_test.rb @@ -80,6 +80,26 @@ def test_cta_is_bulletproof_presentation_tables assert_equal tables, output.scan('role="presentation"').size end + def test_cta_blank_injects_default_rel + output = render('Go') + + assert_includes output, 'target="_blank"' + assert_includes output, 'rel="noopener"' + end + + def test_cta_non_blank_gets_no_rel + output = render('Go') + + refute_includes output, 'rel=' + end + + def test_cta_explicit_rel_wins_without_duplication + output = render('Go') + + assert_includes output, 'rel="noopener noreferrer"' + assert_equal 1, output.scan('rel="').size + end + def test_info_box_reads_border_token ActiveMail.tokens.color(:border, '#445566') output = render('note') diff --git a/test/components/menu_test.rb b/test/components/menu_test.rb index 698d6d2..1a16a93 100644 --- a/test/components/menu_test.rb +++ b/test/components/menu_test.rb @@ -22,6 +22,19 @@ def test_item_preserves_target assert_includes output, 'target="_blank"' end + def test_item_blank_injects_default_rel + output = render('I') + + assert_includes output, 'rel="noopener"' + end + + def test_item_explicit_rel_wins_without_duplication + output = render('I') + + assert_includes output, 'rel="noopener noreferrer"' + assert_equal 1, output.scan('rel="').size + end + def test_item_without_href_emits_no_broken_anchor output = render('Label') diff --git a/test/configuration_test.rb b/test/configuration_test.rb index fd54a88..b10d981 100644 --- a/test/configuration_test.rb +++ b/test/configuration_test.rb @@ -9,9 +9,28 @@ def test_defaults assert_equal :erb, config.template_engine assert_equal 12, config.column_count assert_equal 600, config.container_width + assert_equal 'noopener', config.blank_link_rel assert_empty config.components end + def test_blank_link_rel_override_reflected_in_output + ActiveMail.configuration.blank_link_rel = 'noopener noreferrer' + + assert_includes render(''), 'rel="noopener noreferrer"' + end + + def test_blank_link_rel_nil_disables_injection + ActiveMail.configuration.blank_link_rel = nil + + refute_includes render(''), 'rel=' + end + + def test_blank_link_rel_rejects_invalid + config = ActiveMail::Configuration.new + + assert_raises(TypeError) { config.blank_link_rel = 123 } + end + def test_template_engine_setter_coerces_to_symbol config = ActiveMail::Configuration.new config.template_engine = 'slim' From 7a06747056641b87c400f8c368d6426cab472e9d Mon Sep 17 00:00:00 2001 From: Philippe Date: Wed, 1 Jul 2026 14:23:17 +0200 Subject: [PATCH 2/3] fix: treat blank rel as disabled, escape-test, symmetric float guard --- lib/activemail/components/base.rb | 12 +++++++++--- lib/activemail/configuration.rb | 4 +++- test/components/button_test.rb | 14 ++++++++++++++ test/configuration_test.rb | 9 +++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/lib/activemail/components/base.rb b/lib/activemail/components/base.rb index 3ec6054..7e5f5f1 100644 --- a/lib/activemail/components/base.rb +++ b/lib/activemail/components/base.rb @@ -103,18 +103,24 @@ def combine_attributes(node, extra_classes = nil) [pass_through_attributes(node), %(class="#{escape_attr(classes)}")].join end - # An explicit rel wins over the injected blank_link_rel default. sig { params(node: Nokogiri::XML::Node).returns(String) } def link_attributes(node) target = node.attributes['target']&.value - rel = node.attributes['rel']&.value - rel ||= ActiveMail.configuration.blank_link_rel if target == '_blank' + rel = resolve_rel(node, target) [ target ? %( target="#{escape_attr(target)}") : '', rel ? %( rel="#{escape_attr(rel)}") : '' ].join end + sig { params(node: Nokogiri::XML::Node, target: T.nilable(String)).returns(T.nilable(String)) } + def resolve_rel(node, target) + rel = node.attributes['rel']&.value + # Blank is truthy, so a stray rel="" would silently skip the security default. + rel = nil if rel&.strip&.empty? + rel || (ActiveMail.configuration.blank_link_rel if target == '_blank') + end + # Outlook-safe nested-table structure kept in one place for ') + + assert_includes output, 'rel="noopener"' + assert_equal 1, output.scan('rel="').size + end + + def test_rel_is_escaped + output = render('') + anchor = Nokogiri::HTML.fragment(output).at_css('a') + + assert_equal 'a"b', anchor['rel'] + end + def test_classes_are_merged_with_button output = render('') diff --git a/test/configuration_test.rb b/test/configuration_test.rb index b10d981..8af12ac 100644 --- a/test/configuration_test.rb +++ b/test/configuration_test.rb @@ -31,6 +31,13 @@ def test_blank_link_rel_rejects_invalid assert_raises(TypeError) { config.blank_link_rel = 123 } end + def test_blank_link_rel_empty_string_disables_injection + ActiveMail.configuration.blank_link_rel = ' ' + + assert_nil ActiveMail.configuration.blank_link_rel + refute_includes render(''), 'rel=' + end + def test_template_engine_setter_coerces_to_symbol config = ActiveMail::Configuration.new config.template_engine = 'slim' @@ -77,7 +84,9 @@ def test_dimension_setters_reject_floats_instead_of_truncating config = ActiveMail::Configuration.new assert_raises(TypeError) { config.column_count = 12.9 } + assert_raises(TypeError) { config.container_width = 4.2 } assert_equal 12, config.column_count + assert_equal 600, config.container_width end def test_constructor_rejects_non_positive_dimensions From 6c44bc48d38fbb08e4fd637b6099e46e6a2a59eb Mon Sep 17 00:00:00 2001 From: Philippe Date: Wed, 1 Jul 2026 14:31:03 +0200 Subject: [PATCH 3/3] refacto: drop needless comments --- lib/activemail/components/base.rb | 2 -- lib/activemail/configuration.rb | 2 -- 2 files changed, 4 deletions(-) diff --git a/lib/activemail/components/base.rb b/lib/activemail/components/base.rb index 7e5f5f1..78d1caa 100644 --- a/lib/activemail/components/base.rb +++ b/lib/activemail/components/base.rb @@ -28,7 +28,6 @@ class Base abstract! - # 'rel' is emitted by #link_attributes; passing it through too would duplicate it. IGNORED_ON_PASSTHROUGH = T.let( %w[class id href size large no-expander small target rel up size-sm size-lg style].freeze, T::Array[String] @@ -116,7 +115,6 @@ def link_attributes(node) sig { params(node: Nokogiri::XML::Node, target: T.nilable(String)).returns(T.nilable(String)) } def resolve_rel(node, target) rel = node.attributes['rel']&.value - # Blank is truthy, so a stray rel="" would silently skip the security default. rel = nil if rel&.strip&.empty? rel || (ActiveMail.configuration.blank_link_rel if target == '_blank') end diff --git a/lib/activemail/configuration.rb b/lib/activemail/configuration.rb index 0739842..00b8b71 100644 --- a/lib/activemail/configuration.rb +++ b/lib/activemail/configuration.rb @@ -86,7 +86,6 @@ def self.assert_positive_dimension!(name, value) value end - # TypeError (not ArgumentError) so a non-Integer is a caller mistake, distinct from a bad value. sig { params(name: Symbol, value: T.untyped).returns(Integer) } def self.positive_integer!(name, value) raise TypeError, "#{name} must be an Integer, got #{value.inspect} (#{value.class})" unless value.is_a?(Integer) @@ -198,7 +197,6 @@ def container_width=(value) def blank_link_rel=(value) raise TypeError, "blank_link_rel must be a String or nil, got #{value.inspect} (#{value.class})" unless value.nil? || value.is_a?(String) - # Blank is truthy in Ruby; normalize to nil so "" means "disabled", not rel="". normalized = value&.strip @blank_link_rel = normalized&.empty? ? nil : normalized end