Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions app/models/link_card/card.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ def metadata
private

def request
return unless LinkChecker::Checker.valid_url?(@url)

@tweet ? fetch_tweet : Metadata.new(@url).fetch
end

Expand Down
74 changes: 74 additions & 0 deletions app/models/link_fetcher/fetcher.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# frozen_string_literal: true

require 'net/http'

module LinkFetcher
module Fetcher
class TooManyRedirects < StandardError; end

FETCH_ERRORS = [
SocketError,
Errno::ECONNREFUSED,
Errno::EHOSTUNREACH,
Errno::ETIMEDOUT,
Net::OpenTimeout,
Net::ReadTimeout,
EOFError,
OpenSSL::SSL::SSLError
].freeze

DEFAULT_TIMEOUT = 5
DEFAULT_REDIRECT_LIMIT = 5

module_function

def fetch(url, redirect_limit = DEFAULT_REDIRECT_LIMIT)
raise TooManyRedirects if redirect_limit.negative?
return nil unless LinkChecker::Checker.valid_url?(url)

uri = Addressable::URI.parse(url).normalize
safe_ips = SafeIpResolver.resolve_ips(uri)
return nil if safe_ips.nil?

http = build_http(uri, safe_ips)
response = http.request_get(uri.request_uri)

if response.is_a?(Net::HTTPRedirection)
redirect_url = build_redirect_url(url, response)
return nil unless redirect_url

fetch(redirect_url, redirect_limit - 1)
else
response
end
rescue TooManyRedirects
Rails.logger.info("[LinkFetcher] Too many redirects: #{url}")
nil
rescue *FETCH_ERRORS => e
Rails.logger.warn("[LinkFetcher] #{e.class}: #{e.message}")
nil
end

def build_http(uri, ips)
http = Net::HTTP.new(uri.host, uri.inferred_port)
if uri.scheme == 'https'
http.use_ssl = true
http.verify_mode = OpenSSL::SSL::VERIFY_PEER
end
http.open_timeout = DEFAULT_TIMEOUT
http.read_timeout = DEFAULT_TIMEOUT
http.response_body_encoding = true
Copy link
Copy Markdown
Contributor Author

@mousu-a mousu-a Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http.response_body_encoding = trueとすることで、encodingをHTMLから推測してくれるようになります。

https://github.com/ruby/ruby/blob/master/lib/net/http.rb#L2450
https://github.com/ruby/ruby/blob/master/lib/net/http/response.rb#L372

http.ipaddr = ips.first
http
end

def build_redirect_url(url, response)
location = response['location']
return nil if location.blank?

URI.join(url, location).to_s
rescue URI::InvalidURIError
nil
end
end
end
76 changes: 76 additions & 0 deletions app/models/link_fetcher/safe_ip_resolver.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# frozen_string_literal: true

module LinkFetcher
module SafeIpResolver
BLOCKED_IP_RANGES = [
# ループバック(SSRFで内部サービスに到達可能なためブロック)
IPAddr.new('127.0.0.0/8'),
IPAddr.new('::1/128'),

# プライベートネットワーク(SSRFで社内/VPCリソースに到達可能なためブロック)
IPAddr.new('10.0.0.0/8'),
IPAddr.new('172.16.0.0/12'),
IPAddr.new('192.168.0.0/16'),

# リンクローカル(メタデータAPI等に到達可能なためブロック)
IPAddr.new('169.254.0.0/16'),
IPAddr.new('fe80::/10'),

# IPv6 ユニークローカルアドレス(内部ネットワークへのアクセスを防ぐためブロック)
IPAddr.new('fc00::/7'),

# 旧site-local(廃止済みだが内部ネットワーク扱いのため念の為ブロック)
IPAddr.new('fec0::/10'),

# 無効・未指定アドレス(接続先として無効、且つ挙動が不安定のためブロック)
IPAddr.new('0.0.0.0/8'),
IPAddr.new('::/128'),

# CGNAT(外部から到達すべきでない領域のためブロック)
IPAddr.new('100.64.0.0/10'),

# IPv4-mapped IPv6(IPv4アドレスをIPv6で表現してのフィルタ回避を防ぐためブロック)
IPAddr.new('::ffff:0:0/96')
].freeze
Comment thread
coderabbitai[bot] marked this conversation as resolved.

class NonHttpUriError < StandardError; end
class AddressNotFoundError < StandardError; end
class UnsafeIpError < StandardError; end

module_function

def resolve_ips(uri)
raise NonHttpUriError unless valid_http_uri?(uri)

ips = Resolv.getaddresses(uri.host)
raise AddressNotFoundError if ips.empty?
raise UnsafeIpError unless all_ips_safe?(ips)

ips
Comment thread
coderabbitai[bot] marked this conversation as resolved.
rescue NonHttpUriError => e
Rails.logger.info("[SafeResolver] #{e.class}: scheme=#{uri.scheme} host=#{uri.host}")
nil
rescue AddressNotFoundError => e
Rails.logger.info("[SafeResolver] #{e.class}: host=#{uri.host}")
nil
rescue UnsafeIpError => e
Rails.logger.warn("[SafeResolver] #{e.class}: unsafe ip detected host=#{uri.host}")
nil
end

def valid_http_uri?(uri)
port = uri.port || uri.inferred_port

port.in?([80, 443]) && uri.scheme.downcase.in?(%w[http https])
end
Comment on lines +61 to +65
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "safe_ip_resolver.rb" -type f

Repository: fjordllc/bootcamp

Length of output: 106


🏁 Script executed:

cd app && find . -name "safe_ip_resolver.rb" -type f | head -20

Repository: fjordllc/bootcamp

Length of output: 102


🏁 Script executed:

fd -t f "safe_ip_resolver" 2>/dev/null || find . -name "*safe_ip_resolver*" -type f 2>/dev/null | head -20

Repository: fjordllc/bootcamp

Length of output: 154


🏁 Script executed:

rg -l "safe_ip_resolver" --type rb

Repository: fjordllc/bootcamp

Length of output: 87


🏁 Script executed:

rg "valid_http_uri" -A 20 -B 5 --type rb

Repository: fjordllc/bootcamp

Length of output: 87


🏁 Script executed:

cat -n app/models/link_fetcher/safe_ip_resolver.rb

Repository: fjordllc/bootcamp

Length of output: 2408


🏁 Script executed:

cat -n test/models/link_fetcher/safe_ip_resolver_test.rb

Repository: fjordllc/bootcamp

Length of output: 3184


scheme/host 欠落URIを fail-closed にしてください。

uri.scheme.downcase は scheme がないURLで NoMethodError になり、現在の rescue 対象外です。同様に、uri.host が nil の場合、Resolv.getaddresses(nil) へ進むことになります。これらの無効な状態を検証時にブロックし、false を返してください。

🛡️ 修正例
    def valid_http_uri?(uri)
+     return false if uri.blank?
+
+     scheme = uri.scheme&.downcase
+     return false if scheme.blank? || uri.host.blank?
+
      port = uri.port || uri.inferred_port

-     port.in?([80, 443]) && uri.scheme.downcase.in?(%w[http https])
+     port.in?([80, 443]) && scheme.in?(%w[http https])
    end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/models/link_fetcher/safe_ip_resolver.rb` around lines 56 - 60, The
validation in valid_http_uri? should be fail-closed for missing scheme/host:
explicitly check that uri.scheme and uri.host are present (non-nil) before
calling uri.scheme.downcase or proceeding to DNS resolution, and return false
immediately if either is missing; keep the existing port check (uri.port ||
uri.inferred_port) and scheme validation but perform them only after the
presence checks. Update valid_http_uri? (and any places that call
Resolv.getaddresses with uri.host) so you never call methods on nil (e.g., avoid
uri.scheme.downcase when scheme is nil and avoid passing nil to
Resolv.getaddresses).

Copy link
Copy Markdown
Contributor Author

@mousu-a mousu-a Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#9778 (comment)
このコメントと同様で、リンクの正当性チェックはこのモジュールの責務ではないので、その点に関して過度に防衛的なコードにする必要性はないと考えています。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mousu-a、ご説明ありがとうございます。URIの正当性チェックは上流の LinkFetcher::Fetcher の責務であり、SafeIpResolver はIPの安全性解決のみを担うというコントラクト設計ですね。了解しました。この指摘は取り下げます。


🧠 Learnings used
Learnt from: mousu-a
Repo: fjordllc/bootcamp PR: 9778
File: app/models/link_fetcher/safe_ip_resolver.rb:48-52
Timestamp: 2026-04-21T03:25:37.345Z
Learning: In fjordllc/bootcamp, for link-fetcher safety resolver components like `LinkFetcher::SafeIpResolver`, treat the input contract as: callers must provide a valid, already-parsed URI object. Do not add defensive `nil`/blank checks inside the resolver (e.g., `&.`) unless you also change the upstream contract/validation flow. The resolver’s responsibility is IP safety resolution only; URI/link validity is expected to be enforced upstream (e.g., in `LinkFetcher::Fetcher`).

Learnt from: mousu-a
Repo: fjordllc/bootcamp PR: 9778
File: app/models/link_fetcher/safe_ip_resolver.rb:25-34
Timestamp: 2026-04-21T03:37:11.383Z
Learning: In fjordllc/bootcamp PR `#9778`, `LinkFetcher::SafeIpResolver::BLOCKED_IP_RANGES` intentionally excludes documentation/TEST-NET ranges (192.0.2.0/24, 198.51.100.0/24, 203.0.113.0/24), benchmark ranges (198.18.0.0/15), multicast (224.0.0.0/4), and reserved (240.0.0.0/4) from the blocklist. The module's scope is strictly SSRF safety (blocking access to genuinely reachable internal/sensitive infrastructure), not link card validity. These ranges are excluded because they are not real SSRF vectors in practice (they don't route to internal services, and TCP connections to them are rejected at the OS/network layer).

Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/models/buzz.rb:6-6
Timestamp: 2025-08-30T00:15:39.732Z
Learning: 管理者限定のフォーム入力の場合、SSRFリスクは大幅に軽減されるため、スキームのホワイトリスト(http/httpsのみ)とNokogiri::HTML.parse()の正しい使用法による最小限の修正で十分とする。

Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:131-135
Timestamp: 2025-09-12T21:18:00.834Z
Learning: Rails アップグレードPRにおいて、product_review_not_completed メソッドの webhook URL 設定の shared フォールバック追加も、設定システム全体の変更として別PRで対応すべきである。

Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:19-23
Timestamp: 2025-09-12T21:17:26.029Z
Learning: Rails アップグレードPRにおいて、config_for(:secrets) を使った webhook URL 設定の shared セクション未参照問題は、設定システム全体の変更として別PRで対応すべきである。

Learnt from: masyuko0222
Repo: fjordllc/bootcamp PR: 8934
File: app/controllers/test_feature_toggle_controller.rb:4-6
Timestamp: 2025-07-15T04:26:24.899Z
Learning: テスト用の一時的なコードで、すぐにリバートされる予定の場合、nil安全性などの防御的プログラミングの実装は不要とされることがある。

Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/models/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。

Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:52-56
Timestamp: 2025-09-12T21:17:45.295Z
Learning: Rails upgrade PRにおいて、configuration systemの改善提案(shared configuration fallback等)も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。

Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/models/product.rb:59-61
Timestamp: 2025-09-11T14:47:53.256Z
Learning: Rails アップグレードPRでは、アップグレードに直接関連しない性能改善や機能追加の提案は避けるべき。PRのスコープを維持することが重要で、そのような改善提案は別PRで対応すべきである。

Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。

Learnt from: jun-kondo
Repo: fjordllc/bootcamp PR: 0
File: :0-0
Timestamp: 2025-07-23T20:42:19.974Z
Learning: fjordllc/bootcampプロジェクトでは、hタグ(見出し)の文言は日本語でハードコーディングする方針が確立されており、I18n対応は行わない。例:app/views/welcome/logo.html.slimでh2、h3タグが日本語でハードコーディングされている。

Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 8992
File: app/models/correct_answer_notifier.rb:23-23
Timestamp: 2025-07-30T07:26:36.599Z
Learning: PRのスコープを維持することが重要であり、バグ修正PRにおいて、修正対象以外の機能(例:ルーティングテスト)の追加提案は適切ではない。そのような改善が必要な場合は別PRで対応すべきである。

Learnt from: hirokiej
Repo: fjordllc/bootcamp PR: 8740
File: app/helpers/reports_helper.rb:56-64
Timestamp: 2025-06-29T03:44:15.179Z
Learning: このプロジェクト(fjordllc/bootcamp)では、ja.ymlファイルで一部の単語や文章のみI18n対応されているが、多くのテキストは日本語でハードコーディングされており、完全な国際化対応は行われていない。新しい機能でもI18n対応は不要と判断される。

Learnt from: mousu-a
Repo: fjordllc/bootcamp PR: 8566
File: app/helpers/pair_work_helper.rb:31-33
Timestamp: 2026-01-15T03:20:24.643Z
Learning: In this Ruby project, prefer Numeric#hours (plural) for adding time durations. Use the plural hours method when converting integers to durations (e.g., hour_count.hours). Apply this consistently across the codebase to ensure uniform time calculations and Rails-style duration handling.

Learnt from: kutimiti1234
Repo: fjordllc/bootcamp PR: 9526
File: app/models/regular_event.rb:0-0
Timestamp: 2026-01-19T14:07:11.947Z
Learning: In Rails apps, avoid placing display/filtering logic in models. Keep business/domain logic in models, and handle display-related filtering in controllers. For RegularEvent filtering based on params[:target] (e.g., 'not_finished' or 'all'), implement the logic in the controller (e.g., RegularEventsController) and keep models free of presentation concerns.


def all_ips_safe?(ips)
ip_addrs = ips.map { |ip| IPAddr.new(ip) }
ip_addrs.all? { |ip| safe_ip?(ip) }
end

def safe_ip?(ip_addr)
BLOCKED_IP_RANGES.none? { |ip_range| ip_range.include?(ip_addr) }
end
end
end
13 changes: 8 additions & 5 deletions app/models/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
class Metadata
def initialize(url)
@url = url
@uri = Addressable::URI.parse(url).normalize
end

def uri
@uri ||= Addressable::URI.parse(@url).normalize
end

def fetch
response = Net::HTTP.get_response(@uri)
response.message == 'OK' ? parse(response.body) : nil
response = LinkFetcher::Fetcher.fetch(@url)
response.is_a?(Net::HTTPSuccess) ? parse(response.body) : nil
end

private
Expand All @@ -21,15 +24,15 @@ def parse(html)
title: object.og.title,
description: object.og.description,
images: object.og.image&.url,
site_name: object.og.site_name || @uri.host,
site_name: object.og.site_name || uri.host,
favicon: favicon(html),
url: @url,
site_url: site_url
}
end

def site_url
"#{@uri.scheme}://#{@uri.host}"
"#{uri.scheme}://#{uri.host}"
end

def favicon(html)
Expand Down
67 changes: 66 additions & 1 deletion db/fixtures/reports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ report89:
### detailsとmessage以外のマークダウンにネストしている場合
* @[card](https://bootcamp.fjord.jp)

reported_on: <%= Time.now %>
reported_on: "2026-01-01 01:00:00"

report90:
user: machida
Expand All @@ -505,3 +505,68 @@ report90:
description: |-
たくさんのスタンプがつきました。
reported_on: "2025-10-01 01:00:00"

report91:
user: komagata
title: "リンクカードの動作確認 危険なIPが弾かれているかのテスト"
emotion: 2
description: |-
## 正常系
@[card](https://www.youtube.com/watch?v=8LudKmk7yPM)

## 異常形
## ループバック
@[card](http://127.0.0.1)

@[card](http://127.0.0.1:80)

@[card](http://[::1])

@[card](http://localhost)

## プライベートネットワーク
@[card](http://10.0.0.1)

@[card](http://172.16.0.1)

@[card](http://192.168.0.1)

## リンクローカル
@[card](http://169.254.0.1)

@[card](http://169.254.169.254)

@[card](http://[fe80::1])

## IPv6 ローカル
@[card](http://[fc00::1])

@[card](http://[fec0::1])

## 未指定・ブロードキャスト
@[card](http://0.0.0.0)

@[card](http://255.255.255.255)

## CGNAT
@[card](http://100.64.0.1)

## ドキュメント用・ベンチマーク用
@[card](http://192.0.2.1)

@[card](http://198.51.100.1)

@[card](http://203.0.113.1)

@[card](http://198.18.0.1)

@[card](http://[2001:db8::1])

## マルチキャスト
@[card](http://224.0.0.1)

@[card](http://[ff00::1])

## IPv4-mapped IPv6
@[card](http://[::ffff:127.0.0.1])
reported_on: "2026-01-02 01:00:00"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading
Loading