Skip to content

Commit 6e5fab8

Browse files
committed
Restrict server-side URL validation to allowlisted hosts (SSRF protection)
Server-side HEAD requests for URL validation could be exploited for SSRF attacks against internal networks. This change restricts validation to a trusted allowlist of avatar hosts: - gravatar.com, www.gravatar.com, secure.gravatar.com - avatars.githubusercontent.com Non-allowlisted URLs skip server-side validation entirely and rely on client-side error handling via the avatar-fallback web component. This provides defense in depth: - Allowlisted hosts: Server-side validation (no flicker for broken URLs) - Other hosts: Client-side fallback (may flicker briefly on 404)
1 parent 2cb3c1c commit 6e5fab8

File tree

8 files changed

+112
-67
lines changed

8 files changed

+112
-67
lines changed

app/components/primer/open_project/avatar_with_fallback.rb

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ class AvatarWithFallback < Primer::Beta::Avatar
2222
# Set to false to disable server-side URL validation (useful for tests)
2323
mattr_accessor :validate_urls, default: true
2424

25+
# Allowlist of trusted avatar hosts for server-side validation.
26+
# Only these hosts will be checked via HEAD request to avoid SSRF vulnerabilities.
27+
ALLOWED_AVATAR_HOSTS = %w[
28+
gravatar.com
29+
www.gravatar.com
30+
secure.gravatar.com
31+
avatars.githubusercontent.com
32+
].freeze
33+
2534
# @see
2635
# - https://primer.style/foundations/typography/
2736
# - https://github.com/primer/css/blob/main/src/support/variables/typography.scss
@@ -68,15 +77,18 @@ def require_src_or_alt_arguments(src, alt)
6877
def resolve_src(src)
6978
return @fallback_svg if src.blank?
7079

71-
if validate_urls
72-
return @fallback_svg if absolute_url?(src) && !url_accessible?(src)
80+
if validate_urls && allowed_host?(src) && !url_accessible?(src)
81+
return @fallback_svg
7382
end
7483

7584
src
7685
end
7786

78-
def absolute_url?(url)
79-
url.to_s.match?(%r{\Ahttps?://}i)
87+
def allowed_host?(url)
88+
uri = URI.parse(url)
89+
ALLOWED_AVATAR_HOSTS.include?(uri.host&.downcase)
90+
rescue URI::InvalidURIError
91+
false
8092
end
8193

8294
def url_accessible?(url)

previews/primer/open_project/avatar_with_fallback_preview.rb

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,37 +69,49 @@ def fallback_as_link
6969

7070
# @!group Error Handling (404 Fallback)
7171
#
72-
# @label Broken absolute URL (server-side fallback)
72+
# @label Broken Gravatar (server-side, no flicker)
7373
# @snapshot
74-
def broken_absolute_url
75-
# Absolute URLs are validated server-side via HEAD request
74+
def broken_gravatar_url
75+
# Gravatar URLs are in the allowlist - validated server-side via HEAD request
7676
# If inaccessible, fallback SVG is rendered immediately (no flicker)
7777
render(Primer::OpenProject::AvatarWithFallback.new(
78-
src: "https://example.com/non-existent-avatar.png",
79-
alt: "User With Missing Avatar",
78+
src: "https://gravatar.com/avatar/nonexistent123",
79+
alt: "User With Missing Gravatar",
8080
unique_id: 42
8181
))
8282
end
8383

84-
# @label Multiple broken absolute URLs (server-side)
85-
def multiple_broken_absolute_urls
84+
# @label Multiple broken Gravatars (server-side)
85+
def multiple_broken_gravatar_urls
8686
render_with_template(locals: {})
8787
end
8888

89+
# @label Broken non-allowlisted URL (client-side fallback)
90+
# @snapshot
91+
def broken_non_allowlisted_url
92+
# Non-allowlisted hosts skip server-side validation (SSRF protection)
93+
# Client-side JS handles the error and swaps to fallback SVG (may flicker)
94+
render(Primer::OpenProject::AvatarWithFallback.new(
95+
src: "https://example.com/non-existent-avatar.png",
96+
alt: "User With Missing Avatar",
97+
unique_id: 43
98+
))
99+
end
100+
89101
# @label Broken relative URL (client-side fallback)
90102
# @snapshot
91103
def broken_relative_url
92104
# Relative URLs cannot be validated server-side (no host context)
93-
# Client-side JS handles the error and swaps to fallback SVG
105+
# Client-side JS handles the error and swaps to fallback SVG (may flicker)
94106
render(Primer::OpenProject::AvatarWithFallback.new(
95107
src: "/non-existent-avatar.png",
96108
alt: "User With Missing Avatar",
97-
unique_id: 43
109+
unique_id: 44
98110
))
99111
end
100112

101-
# @label Multiple broken relative URLs (client-side)
102-
def multiple_broken_relative_urls
113+
# @label Multiple broken non-allowlisted URLs (client-side)
114+
def multiple_broken_non_allowlisted_urls
103115
render_with_template(locals: {})
104116
end
105117
#
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<div class="d-flex gap-3 flex-items-center">
2+
<p class="color-fg-muted">Gravatar URLs are validated server-side. Broken URLs render fallback immediately (no flicker):</p>
3+
</div>
4+
<div class="d-flex gap-3 flex-items-center mt-2">
5+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "https://gravatar.com/avatar/nonexistent-alice", alt: "Alice Johnson", unique_id: 10)) %>
6+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "https://gravatar.com/avatar/nonexistent-bob", alt: "Bob Smith", unique_id: 20)) %>
7+
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "https://secure.gravatar.com/avatar/nonexistent-charlie", alt: "Charlie Brown", unique_id: 30)) %>
8+
</div>

previews/primer/open_project/avatar_with_fallback_preview/multiple_broken_absolute_urls.html.erb renamed to previews/primer/open_project/avatar_with_fallback_preview/multiple_broken_non_allowlisted_urls.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<div class="d-flex gap-3 flex-items-center">
2-
<p class="color-fg-muted">Absolute URLs are validated server-side. Broken URLs render fallback immediately (no flicker):</p>
2+
<p class="color-fg-muted">Non-allowlisted URLs skip server-side validation (SSRF protection). Client-side JS handles errors (may flicker):</p>
33
</div>
44
<div class="d-flex gap-3 flex-items-center mt-2">
55
<%= render(Primer::OpenProject::AvatarWithFallback.new(src: "https://example.com/missing/alice.png", alt: "Alice Johnson", unique_id: 10)) %>

previews/primer/open_project/avatar_with_fallback_preview/multiple_broken_relative_urls.html.erb

Lines changed: 0 additions & 8 deletions
This file was deleted.

test/components/open_project/avatar_stack_test.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,6 @@
55
class PrimerOpenProjectAvatarStackTest < Minitest::Test
66
include Primer::ComponentTestHelpers
77

8-
def setup
9-
@original_validate_urls = Primer::OpenProject::AvatarWithFallback.validate_urls
10-
Primer::OpenProject::AvatarWithFallback.validate_urls = false
11-
end
12-
13-
def teardown
14-
Primer::OpenProject::AvatarWithFallback.validate_urls = @original_validate_urls
15-
end
16-
178
def test_renders_with_image_avatars
189
render_inline(Primer::OpenProject::AvatarStack.new) do |component|
1910
component.with_avatar_with_fallback(src: "https://github.com/github.png", alt: "@github")

test/components/open_project/avatar_with_fallback_test.rb

Lines changed: 61 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,11 @@
11
# frozen_string_literal: true
22

33
require "components/test_helper"
4+
require "webmock/minitest"
45

56
class PrimerOpenProjectAvatarWithFallbackTest < Minitest::Test
67
include Primer::ComponentTestHelpers
78

8-
def setup
9-
# Disable URL validation by default in tests to avoid HTTP requests
10-
@original_validate_urls = Primer::OpenProject::AvatarWithFallback.validate_urls
11-
Primer::OpenProject::AvatarWithFallback.validate_urls = false
12-
end
13-
14-
def teardown
15-
Primer::OpenProject::AvatarWithFallback.validate_urls = @original_validate_urls
16-
end
17-
189
def test_renders_image_avatar_with_src
1910
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://github.com/github.png", alt: "github"))
2011

@@ -41,14 +32,63 @@ def test_image_avatar_error_handling_setup
4132
assert_includes svg_content, ">AJ<", "Fallback SVG should contain initials 'AJ'"
4233
end
4334

44-
def test_falls_back_when_absolute_url_is_inaccessible
35+
def test_skips_validation_for_non_allowlisted_hosts
36+
Primer::OpenProject::AvatarWithFallback.validate_urls = true
37+
# url_accessible? should NOT be called for non-allowlisted hosts
38+
Primer::OpenProject::AvatarWithFallback.any_instance.expects(:url_accessible?).never
39+
40+
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://example.com/avatar.png", alt: "Test User", unique_id: 43))
41+
42+
# Should render the original URL (no server-side validation for non-allowlisted hosts)
43+
assert_selector("avatar-fallback") do
44+
assert_selector("img.avatar[src='https://example.com/avatar.png']")
45+
end
46+
end
47+
48+
def test_handles_invalid_uri_in_allowed_host_check
4549
Primer::OpenProject::AvatarWithFallback.validate_urls = true
46-
Primer::OpenProject::AvatarWithFallback.any_instance.stubs(:url_accessible?).returns(false)
4750

48-
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://broken.example.com/avatar.png", alt: "Test User", unique_id: 42))
51+
# Invalid URI should not raise, just skip validation
52+
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "not a valid uri %%", alt: "Test User", unique_id: 45))
4953

50-
# Should render fallback SVG when URL is inaccessible
51-
assert_selector("avatar-fallback[data-unique-id='42']") do
54+
# Should render the original URL (invalid URI treated as non-allowlisted)
55+
assert_selector("avatar-fallback") do
56+
assert_selector("img.avatar[src='not a valid uri %%']")
57+
end
58+
end
59+
60+
def test_url_accessible_returns_true_for_successful_head_request
61+
Primer::OpenProject::AvatarWithFallback.validate_urls = true
62+
stub_request(:head, "https://gravatar.com/avatar/exists").to_return(status: 200)
63+
64+
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://gravatar.com/avatar/exists", alt: "Test User", unique_id: 46))
65+
66+
# Should render original URL when HEAD request succeeds
67+
assert_selector("avatar-fallback") do
68+
assert_selector("img.avatar[src='https://gravatar.com/avatar/exists']")
69+
end
70+
end
71+
72+
def test_url_accessible_returns_false_for_404_response
73+
Primer::OpenProject::AvatarWithFallback.validate_urls = true
74+
stub_request(:head, "https://gravatar.com/avatar/notfound").to_return(status: 404)
75+
76+
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://gravatar.com/avatar/notfound", alt: "Test User", unique_id: 47))
77+
78+
# Should render fallback SVG when HEAD request returns 404
79+
assert_selector("avatar-fallback") do
80+
assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
81+
end
82+
end
83+
84+
def test_url_accessible_returns_false_on_network_error
85+
Primer::OpenProject::AvatarWithFallback.validate_urls = true
86+
stub_request(:head, "https://gravatar.com/avatar/timeout").to_timeout
87+
88+
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://gravatar.com/avatar/timeout", alt: "Test User", unique_id: 48))
89+
90+
# Should render fallback SVG when network error occurs
91+
assert_selector("avatar-fallback") do
5292
assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
5393
end
5494
end
@@ -76,12 +116,6 @@ def test_defaults_to_size_20
76116
assert_selector("img.avatar[size=20][height=20][width=20]")
77117
end
78118

79-
def test_fallback_defaults_to_size_20
80-
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "Test User"))
81-
82-
assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
83-
end
84-
85119
def test_falls_back_when_size_isn_t_valid
86120
without_fetch_or_fallback_raises do
87121
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://github.com/github.png", alt: "github", size: 1_000_000_000))
@@ -171,15 +205,6 @@ def test_fallback_with_unique_id_in_data_attribute
171205
end
172206
end
173207

174-
def test_fallback_without_unique_id
175-
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "Test User"))
176-
177-
# Should still render, just without unique_id data attribute
178-
assert_selector("avatar-fallback[data-alt-text='Test User']") do
179-
assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
180-
end
181-
end
182-
183208
def test_adds_custom_classes
184209
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://github.com/github.png", alt: "github", classes: "custom-class"))
185210

@@ -208,12 +233,14 @@ def test_raises_when_both_src_and_alt_are_blank
208233
assert_includes(error.message, "`src` or `alt` is required")
209234
end
210235

211-
def test_fallback_with_single_word_name
236+
def test_fallback_extracts_single_initial_from_single_word_name
212237
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "Alice"))
213238

214-
assert_selector("avatar-fallback") do
215-
assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
216-
end
239+
fallback_wrapper = page.find("avatar-fallback")
240+
fallback_src = fallback_wrapper["data-fallback-src"]
241+
svg_content = Base64.decode64(fallback_src.sub("data:image/svg+xml;base64,", ""))
242+
243+
assert_includes svg_content, ">A<", "Single word name should produce single initial 'A'"
217244
end
218245

219246
def test_status

test/test_helper.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,8 @@
3232

3333
require File.expand_path("../demo/config/environment.rb", __dir__)
3434

35+
# Disable server-side URL validation in tests to avoid HTTP requests
36+
Primer::OpenProject::AvatarWithFallback.validate_urls = false
37+
3538
# used by SelectPanel
3639
Mime::Type.register "text/fragment+html", :html_fragment

0 commit comments

Comments
 (0)