Files
TJ Paulson df201f3ee4 fix: security review — dashboard XSS, private-mode federation gap, Matrix bridge resilience (#839)
* fix(web): escape untrusted mesh fields in dashboard to close DOM XSS

Security review found user-controlled mesh data reaching innerHTML unescaped
(upstream-originating; worth a PR back to l5yth/potato-mesh):

- High: channel_name was interpolated raw by formatChatChannelTag and rendered
  via innerHTML on the node-detail page and map overlays, so a crafted channel
  name (any radio can set one) executed script for every viewer. Now escaped
  with the canonical escapeHtml.
- Defense-in-depth: node_id, role, and hw_model in the node table are now
  escaped for consistency with sibling cells.

Regression test added for the channel-name escape. Full JS suite: 1448 pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(web): gate POST /api/instances by federation mode + hardening

Security review findings in the Sinatra app (upstream-originating; worth a PR
back to l5yth/potato-mesh):

- High: POST /api/instances performed outbound federation fetches and DB writes
  with no federation/private guard, so a PRIVATE=1 or FEDERATION=0 node still
  acted on unsolicited signed announcements. Added `halt 404 unless
  federation_enabled?`, mirroring the existing GET /api/instances guard.
- Low: the JSON-LD <script> block now escapes </script> breakout characters.
- Low: federation peer fetches now enforce a max response size
  (REMOTE_INSTANCE_MAX_RESPONSE_BYTES, default 8 MiB) to bound memory.

Regression spec added for the federation-gate (private + FEDERATION=0 cases).

NOTE: rspec could not be run locally (this host has only Ruby 2.6; the project
needs >=3.0 and no Docker was available). Changes are `ruby -c` syntax-checked
and reviewed against the mirrored guard; the ruby.yml CI workflow must confirm
the spec on push/PR before merge.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(matrix): stop silent message loss + add HTTP timeouts + log reg failures

Security/resilience review of the Matrix bridge (upstream-originating; worth a
PR back to l5yth/potato-mesh):

- High: within a poll batch, a later message that forwarded successfully
  advanced the in-memory rx_time watermark past an EARLIER message that failed,
  so the failed message was never refetched -> silent permanent loss. poll_once
  now breaks at the first failure, committing only the contiguous successful
  prefix; the failed message and everything after it are retried in order next
  poll (no reordering, no duplicates). Regression test added and proven to bite.
- High: the shared reqwest client had no timeouts, so a hung homeserver/API
  stalled the single-threaded poll loop (and startup health checks) forever.
  Added timeout(30s) + connect_timeout(10s).
- Medium: ensure_user_registered swallowed all non-success responses silently;
  it now warns (status + body) on anything other than the expected
  already-registered case, so a bad as_token is diagnosable.

cargo test: 118 passed; cargo fmt clean. Tradeoff: a permanently-failing
message now blocks its batch (retried each poll) rather than being lost -- safer
than silent loss; a skip-after-N/dead-letter follow-up is possible if needed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: address Copilot review on the security PR (Matrix errcode + spec hygiene)

- matrix.rs: ensure_user_registered now treats only the specific M_USER_IN_USE
  errcode as "already registered", instead of any 400 Bad Request, so real 400
  failures (malformed request, config issues) reach the diagnostic warn branch.
  Updated the M_USER_IN_USE test to send the errcode body and added a test that
  a non-M_USER_IN_USE 400 is not suppressed. cargo test: 119 passing.
- app_spec.rb: the private-mode POST /api/instances example now restores
  ENV["PRIVATE"] via an around/ensure block (mirroring the FEDERATION pattern),
  so private mode can't leak into later specs and cause order-dependent failures.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: implement Copilot follow-ups — streaming size cap + bounded skip

- instance_fetcher.rb: perform_single_http_request now uses the block form of
  Net::HTTP#request + response.read_body to enforce the response-size cap
  INCREMENTALLY, aborting as soon as the limit is exceeded, instead of letting
  the non-block form buffer the whole body into memory first. Updated the two
  federation_spec mocks to the block/streaming form and added a size-cap test.
- main.rs (Matrix bridge): a permanently-failing ("poison") message no longer
  blocks the batch forever. Consecutive per-message failures are counted
  in-memory; after MAX_FORWARD_ATTEMPTS (5) the message is skipped (advanced
  past, with a warning) so later messages make progress. Transient failures
  still stop-and-retry in order (no silent loss, no reordering). Added a
  skip-after-N regression test; the original watermark test still passes.
  cargo test: 120 passing, cargo fmt clean.

Ruby: instance_fetcher.rb is `ruby -c` clean; the spec exercises the new paths
but rspec/`ruby -c` can't run locally (Ruby 2.6 here can't parse the repo's
3.1+ syntax) — the ruby.yml CI is the gate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(web): fix private-mode /api/instances spec to actually enable private mode

Running the suite locally (Ruby 4.0) revealed the private-mode POST /api/instances
example never entered private mode: the top-level `before` hook deletes
ENV["PRIVATE"] before each example, so toggling it (even via an around block)
had no effect and the request returned 201 instead of 404.

Switch to stubbing PotatoMesh::Config.private_mode_enabled? => true, the pattern
the rest of the suite uses for private-mode cases. This makes the test actually
exercise the guard AND removes the ENV manipulation entirely (so there is no
cross-spec ENV leak to worry about). Full web suite: 1470 examples, 0 failures;
rufo clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-05 22:27:37 +02:00

171 lines
7.3 KiB
Ruby

# Copyright © 2025-26 l5yth & contributors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# frozen_string_literal: true
module PotatoMesh
module App
module Federation
# Execute a GET request against the supplied federation URI, cycling
# through resolved IP addresses when a transport-level connection
# failure occurs.
#
# DNS resolution is performed once and the resulting addresses are
# sorted with IPv4 first via {sort_addresses_for_connection}. Each
# address is attempted sequentially; when a connection-level error
# (refused, unreachable, timeout) is raised the next address is tried.
# Non-connection errors (SSL failures, HTTP-level errors) are raised
# immediately without trying further addresses.
#
# @param uri [URI::Generic] target endpoint to request.
# @return [String] raw HTTP response body on success.
# @raise [InstanceFetchError] when all addresses are exhausted or a
# non-retryable error occurs.
def perform_instance_http_request(uri)
raise InstanceFetchError, "federation shutdown requested" if federation_shutdown_requested?
remote_addresses = sort_addresses_for_connection(resolve_remote_ip_addresses(uri))
addresses = remote_addresses.empty? ? [nil] : remote_addresses
last_error = nil
addresses.each do |address|
break if federation_shutdown_requested?
begin
return perform_single_http_request(uri, ip_address: address&.to_s)
rescue InstanceFetchError => e
if connection_refused_or_unreachable?(e)
last_error = e
else
raise
end
end
end
raise last_error || InstanceFetchError.new("all resolved addresses failed")
rescue ArgumentError, SocketError => e
# +resolve_remote_ip_addresses+ runs the DNS lookup before the wrapped
# HTTP attempt: a blank/restricted host raises ArgumentError, and an
# unresolvable domain raises Socket::ResolutionError (a SocketError).
# Both are converted to InstanceFetchError so every fetch_instance_json
# caller rejects the peer gracefully instead of letting a raw resolution
# error escape as a 500. (HTTP-attempt errors are already wrapped inside
# perform_single_http_request, so this never masks a live connection.)
raise_instance_fetch_error(e)
end
# Execute a single HTTP GET request against the supplied URI, optionally
# pinning the connection to a specific IP address.
#
# @param uri [URI::Generic] target endpoint.
# @param ip_address [String, nil] resolved IP address to pin the
# connection to, or +nil+ to let {build_remote_http_client} resolve.
# @return [String] raw HTTP response body.
# @raise [InstanceFetchError] when the request fails.
def perform_single_http_request(uri, ip_address: nil)
http = build_remote_http_client(uri, ip_address: ip_address)
Timeout.timeout(PotatoMesh::Config.remote_instance_request_timeout) do
http.start do |connection|
request = build_federation_http_request(Net::HTTP::Get, uri)
# Stream the response with the block form of +request+ so the size
# cap (mirroring the inbound +read_json_body+ ceiling) is enforced
# *incrementally*. The non-block form buffers the whole body into
# memory before we could inspect it, so a malicious/oversized peer
# could still cause a large allocation before we raise. Reading in
# chunks lets us abort as soon as the limit is exceeded.
max_bytes = PotatoMesh::Config.remote_instance_max_response_bytes
body = nil
connection.request(request) do |response|
unless response.is_a?(Net::HTTPSuccess)
raise InstanceHttpResponseError, "unexpected response #{response.code}"
end
buffer = +""
response.read_body do |chunk|
buffer << chunk
if buffer.bytesize > max_bytes
raise InstanceHttpResponseError,
"response exceeds maximum size of #{max_bytes} bytes"
end
end
body = buffer
end
body
end
end
rescue InstanceHttpResponseError
# Reached the peer at the HTTP layer; do not wrap so callers can
# distinguish "peer responded with non-2xx" from "transport failure".
raise
rescue StandardError => e
raise_instance_fetch_error(e)
end
# Build a human readable error message for a failed instance request.
#
# @param error [StandardError] failure raised while performing the request.
# @return [String] description including the error class when necessary.
def instance_fetch_error_message(error)
message = error.message.to_s.strip
class_name = error.class.name || error.class.to_s
return class_name if message.empty?
message.include?(class_name) ? message : "#{class_name}: #{message}"
end
# Raise an InstanceFetchError that preserves the original context.
#
# @param error [StandardError] failure raised while performing the request.
# @return [void]
def raise_instance_fetch_error(error)
message = instance_fetch_error_message(error)
wrapped = InstanceFetchError.new(message)
wrapped.set_backtrace(error.backtrace)
raise wrapped
end
# Fetch and JSON-decode a federation document from a peer.
#
# @param domain [String] peer hostname.
# @param path [String] request path.
# @return [Array(Object, URI::Generic | Array<String>)] decoded payload
# plus the successful URI, or +[nil, errors]+ when every candidate fails.
def fetch_instance_json(domain, path)
return [nil, ["federation shutdown requested"]] if federation_shutdown_requested?
errors = []
instance_uri_candidates(domain, path).each do |uri|
break if federation_shutdown_requested?
begin
body = perform_instance_http_request(uri)
return [JSON.parse(body), uri] if body
rescue JSON::ParserError => e
errors << "#{uri}: invalid JSON (#{e.message})"
rescue InstanceHttpResponseError => e
# Peer answered at the HTTP layer (e.g. 4xx/5xx). Falling back to
# the next transport candidate (http:// after https://) adds noise
# without adding any chance of success — stop here.
errors << "#{uri}: #{e.message}"
break
rescue InstanceFetchError => e
errors << "#{uri}: #{e.message}"
end
end
[nil, errors]
end
end
end
end