Skip to content

Commit fdd9db0

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 fdd9db0

File tree

6 files changed

+64
-26
lines changed

6 files changed

+64
-26
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_with_fallback_test.rb

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,32 @@ def test_image_avatar_error_handling_setup
4141
assert_includes svg_content, ">AJ<", "Fallback SVG should contain initials 'AJ'"
4242
end
4343

44-
def test_falls_back_when_absolute_url_is_inaccessible
44+
def test_falls_back_when_allowed_host_url_is_inaccessible
4545
Primer::OpenProject::AvatarWithFallback.validate_urls = true
4646
Primer::OpenProject::AvatarWithFallback.any_instance.stubs(:url_accessible?).returns(false)
4747

48-
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://broken.example.com/avatar.png", alt: "Test User", unique_id: 42))
48+
# Uses gravatar.com which is in the allowlist
49+
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://gravatar.com/avatar/nonexistent", alt: "Test User", unique_id: 42))
4950

5051
# Should render fallback SVG when URL is inaccessible
5152
assert_selector("avatar-fallback[data-unique-id='42']") do
5253
assert_selector("img.avatar[src^='data:image/svg+xml;base64,']")
5354
end
5455
end
5556

57+
def test_skips_validation_for_non_allowlisted_hosts
58+
Primer::OpenProject::AvatarWithFallback.validate_urls = true
59+
# url_accessible? should NOT be called for non-allowlisted hosts
60+
Primer::OpenProject::AvatarWithFallback.any_instance.expects(:url_accessible?).never
61+
62+
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: "https://example.com/avatar.png", alt: "Test User", unique_id: 43))
63+
64+
# Should render the original URL (no server-side validation for non-allowlisted hosts)
65+
assert_selector("avatar-fallback") do
66+
assert_selector("img.avatar[src='https://example.com/avatar.png']")
67+
end
68+
end
69+
5670
def test_renders_fallback_when_src_is_nil
5771
render_inline(Primer::OpenProject::AvatarWithFallback.new(src: nil, alt: "OpenProject Admin"))
5872

0 commit comments

Comments
 (0)