Harden federation domain validation and tests (#347)

* Harden federation domain validation and tests

* Preserve domain casing for signature verification

* Forward sanitize helper keyword argument

* Handle mixed-case domains during signature verification

web/lib/potato_mesh/application/federation.rb

@@ -22,6 +22,25 @@ module PotatoMesh
         raise "INSTANCE_DOMAIN could not be determined"
       end
 
+      # Determine whether the local instance should persist its own record.
+      #
+      # @param domain [String, nil] candidate domain for the running instance.
+      # @return [Array(Boolean, String, nil)] tuple containing a decision flag and an optional reason.
+      def self_instance_registration_decision(domain)
+        source = app_constant(:INSTANCE_DOMAIN_SOURCE)
+        return [false, "INSTANCE_DOMAIN source is #{source}"] unless source == :environment
+
+        sanitized = sanitize_instance_domain(domain)
+        return [false, "INSTANCE_DOMAIN missing or invalid"] unless sanitized
+
+        ip = ip_from_domain(sanitized)
+        if ip && restricted_ip_address?(ip)
+          return [false, "INSTANCE_DOMAIN resolves to restricted IP"]
+        end
+
+        [true, nil]
+      end
+
       def self_instance_attributes
         domain = self_instance_domain
         last_update = latest_node_update_timestamp || Time.now.to_i
@@ -68,43 +87,68 @@ module PotatoMesh
       def ensure_self_instance_record!
         attributes = self_instance_attributes
         signature = sign_instance_attributes(attributes)
-        db = open_database
-        upsert_instance_record(db, attributes, signature)
-        debug_log(
-          "Registered self instance record",
-          context: "federation.instances",
-          domain: attributes[:domain],
-          instance_id: attributes[:id],
-        )
+        db = nil
+        allowed, reason = self_instance_registration_decision(attributes[:domain])
+        if allowed
+          db = open_database
+          upsert_instance_record(db, attributes, signature)
+          debug_log(
+            "Registered self instance record",
+            context: "federation.instances",
+            domain: attributes[:domain],
+            instance_id: attributes[:id],
+          )
+        else
+          debug_log(
+            "Skipped self instance registration",
+            context: "federation.instances",
+            domain: attributes[:domain],
+            reason: reason,
+          )
+        end
         [attributes, signature]
       ensure
         db&.close
       end
 
       def federation_target_domains(self_domain)
-        domains = Set.new
+        normalized_self = sanitize_instance_domain(self_domain)&.downcase
+        ordered = []
+        seen = Set.new
+
         PotatoMesh::Config.federation_seed_domains.each do |seed|
-          sanitized = sanitize_instance_domain(seed)
-          domains << sanitized.downcase if sanitized
+          sanitized = sanitize_instance_domain(seed)&.downcase
+          next unless sanitized
+          next if normalized_self && sanitized == normalized_self
+          next if seen.include?(sanitized)
+
+          ordered << sanitized
+          seen << sanitized
         end
 
         db = open_database(readonly: true)
         db.results_as_hash = false
-        rows = with_busy_retry { db.execute("SELECT domain FROM instances WHERE domain IS NOT NULL AND TRIM(domain) != ''") }
+        rows = with_busy_retry {
+          db.execute("SELECT domain FROM instances WHERE domain IS NOT NULL AND TRIM(domain) != ''")
+        }
         rows.flatten.compact.each do |raw_domain|
-          sanitized = sanitize_instance_domain(raw_domain)
-          domains << sanitized.downcase if sanitized
+          sanitized = sanitize_instance_domain(raw_domain)&.downcase
+          next unless sanitized
+          next if normalized_self && sanitized == normalized_self
+          next if seen.include?(sanitized)
+
+          ordered << sanitized
+          seen << sanitized
         end
-        if self_domain
-          domains.delete(self_domain.downcase)
-        end
-        domains.to_a
+        ordered
       rescue SQLite3::Exception
-        domains =
-          PotatoMesh::Config.federation_seed_domains.map do |seed|
-            sanitize_instance_domain(seed)&.downcase
-          end.compact
-        self_domain ? domains.reject { |domain| domain == self_domain.downcase } : domains
+        fallback = PotatoMesh::Config.federation_seed_domains.filter_map do |seed|
+          candidate = sanitize_instance_domain(seed)&.downcase
+          next if normalized_self && candidate == normalized_self
+
+          candidate
+        end
+        fallback.uniq
       ensure
         db&.close
       end
@@ -435,14 +479,13 @@ module PotatoMesh
         latest = nodes.filter_map do |node|
           next unless node.is_a?(Hash)
 
-          timestamps = []
-          timestamps << coerce_integer(node["last_heard"])
-          timestamps << coerce_integer(node["position_time"])
-          timestamps << coerce_integer(node["first_heard"])
-          timestamps.compact.max
+          last_heard_values = []
+          last_heard_values << coerce_integer(node["last_heard"])
+          last_heard_values << coerce_integer(node["lastHeard"])
+          last_heard_values.compact.max
         end.compact.max
 
-        return [false, "missing recent node updates"] unless latest
+        return [false, "missing last_heard data"] unless latest
 
         cutoff = Time.now.to_i - PotatoMesh::Config.remote_instance_max_node_age
         return [false, "node data is stale"] if latest < cutoff
@@ -451,6 +494,34 @@ module PotatoMesh
       end
 
       def upsert_instance_record(db, attributes, signature)
+        sanitized_domain = sanitize_instance_domain(attributes[:domain])
+        raise ArgumentError, "invalid domain" unless sanitized_domain
+
+        ip = ip_from_domain(sanitized_domain)
+        if ip && restricted_ip_address?(ip)
+          raise ArgumentError, "restricted domain"
+        end
+
+        normalized_domain = sanitized_domain
+        existing_id = with_busy_retry do
+          db.get_first_value(
+            "SELECT id FROM instances WHERE domain = ?",
+            normalized_domain,
+          )
+        end
+        if existing_id && existing_id != attributes[:id]
+          with_busy_retry do
+            db.execute("DELETE FROM instances WHERE id = ?", existing_id)
+          end
+          debug_log(
+            "Removed conflicting instance by domain",
+            context: "federation.instances",
+            domain: normalized_domain,
+            replaced_id: existing_id,
+            incoming_id: attributes[:id],
+          )
+        end
+
         sql = <<~SQL
           INSERT INTO instances (
             id, domain, pubkey, name, version, channel, frequency,
@@ -472,7 +543,7 @@ module PotatoMesh
 
         params = [
           attributes[:id],
-          attributes[:domain],
+          normalized_domain,
           attributes[:pubkey],
           attributes[:name],
           attributes[:version],

web/lib/potato_mesh/application/helpers.rb

@@ -53,9 +53,10 @@ module PotatoMesh
       # Proxy for {PotatoMesh::Sanitizer.sanitize_instance_domain}.
       #
       # @param value [Object] candidate domain string.
+      # @param downcase [Boolean] whether to force lowercase normalisation.
       # @return [String, nil] canonical domain or nil.
-      def sanitize_instance_domain(value)
-        PotatoMesh::Sanitizer.sanitize_instance_domain(value)
+      def sanitize_instance_domain(value, downcase: true)
+        PotatoMesh::Sanitizer.sanitize_instance_domain(value, downcase: downcase)
       end
 
       # Proxy for {PotatoMesh::Sanitizer.instance_domain_host}.

web/lib/potato_mesh/application/identity.rb

@@ -170,11 +170,13 @@ module PotatoMesh
       # @return [Array(String, String)] pair of JSON output and base64 signature.
       def build_well_known_document
         last_update = latest_node_update_timestamp
+        domain_value = sanitize_instance_domain(app_constant(:INSTANCE_DOMAIN))
+
         payload = {
           publicKey: app_constant(:INSTANCE_PUBLIC_KEY_PEM),
           name: sanitized_site_name,
           version: app_constant(:APP_VERSION),
-          domain: app_constant(:INSTANCE_DOMAIN),
+          domain: domain_value,
           lastUpdate: last_update,
         }
 
@@ -236,9 +238,7 @@ module PotatoMesh
         return nil unless File.exist?(PotatoMesh::Config.db_path)
 
         db = open_database(readonly: true)
-        value = db.get_first_value(
-          "SELECT MAX(COALESCE(last_heard, first_heard, position_time)) FROM nodes",
-        )
+        value = db.get_first_value("SELECT MAX(last_heard) FROM nodes")
         value&.to_i
       rescue SQLite3::Exception
         nil

web/lib/potato_mesh/application/routes/ingest.rb

@@ -84,7 +84,10 @@ module PotatoMesh
             end
 
             id = string_or_nil(payload["id"]) || string_or_nil(payload["instanceId"])
-            domain = sanitize_instance_domain(payload["domain"])
+            raw_domain = sanitize_instance_domain(payload["domain"], downcase: false)
+            # Normalise the domain for persistence while retaining the caller's
+            # original casing for signature verification fallbacks.
+            normalized_domain = sanitize_instance_domain(raw_domain)
             pubkey = sanitize_public_key_pem(payload["pubkey"])
             name = string_or_nil(payload["name"])
             version = string_or_nil(payload["version"])
@@ -99,7 +102,7 @@ module PotatoMesh
 
             attributes = {
               id: id,
-              domain: domain,
+              domain: normalized_domain,
               pubkey: pubkey,
               name: name,
               version: version,
@@ -120,11 +123,21 @@ module PotatoMesh
               halt 400, { error: "missing required fields" }.to_json
             end
 
-            unless verify_instance_signature(attributes, signature, attributes[:pubkey])
+            signature_valid = verify_instance_signature(attributes, signature, attributes[:pubkey])
+            # Some remote peers sign payloads using a canonicalised lowercase
+            # domain while still sending a mixed-case domain. Retry signature
+            # verification with the original casing when the first attempt
+            # fails to maximise interoperability.
+            if !signature_valid && raw_domain && normalized_domain && raw_domain.casecmp?(normalized_domain) && raw_domain != normalized_domain
+              alternate_attributes = attributes.merge(domain: raw_domain)
+              signature_valid = verify_instance_signature(alternate_attributes, signature, attributes[:pubkey])
+            end
+
+            unless signature_valid
               warn_log(
                 "Instance registration rejected",
                 context: "ingest.register",
-                domain: attributes[:domain],
+                domain: raw_domain || attributes[:domain],
                 reason: "invalid signature",
               )
               halt 400, { error: "invalid signature" }.to_json

web/lib/potato_mesh/sanitizer.rb

@@ -41,8 +41,9 @@ module PotatoMesh
     # rules. This rejects whitespace, path separators, and trailing dots.
     #
     # @param value [String, Object, nil] candidate domain name.
+    # @param downcase [Boolean] whether to force the result to lowercase.
     # @return [String, nil] canonical domain value or +nil+ when invalid.
-    def sanitize_instance_domain(value)
+    def sanitize_instance_domain(value, downcase: true)
       host = string_or_nil(value)
       return nil unless host
 
@@ -51,7 +52,7 @@ module PotatoMesh
       return nil if trimmed.empty?
       return nil if trimmed.match?(%r{[\s/\\@]})
 
-      trimmed
+      downcase ? trimmed.downcase : trimmed
     end
 
     # Extract the host component from a potentially bracketed domain literal.

web/spec/app_spec.rb

@@ -78,6 +78,15 @@ RSpec.describe "Potato Mesh Sinatra app" do
     ensure_self_instance_record!
   end
 
+  # Retrieve the number of rows stored in the instances table.
+  #
+  # @return [Integer] count of stored instance records.
+  def instance_count
+    with_db(readonly: true) do |db|
+      db.get_first_value("SELECT COUNT(*) FROM instances").to_i
+    end
+  end
+
   # Build a hash excluding entries whose values are nil.
   #
   # @param hash [Hash] collection filtered for nil values.
@@ -739,6 +748,176 @@ RSpec.describe "Potato Mesh Sinatra app" do
         end
       end
     end
+
+    describe ".self_instance_registration_decision" do
+      let(:domain) { "spec.mesh.test" }
+
+      it "rejects registration when the domain source is not the environment" do
+        stub_const("PotatoMesh::Application::INSTANCE_DOMAIN_SOURCE", :reverse_dns) do
+          allowed, reason = application_class.self_instance_registration_decision(domain)
+
+          expect(allowed).to be(false)
+          expect(reason).to eq("INSTANCE_DOMAIN source is reverse_dns")
+        end
+      end
+
+      it "rejects registration when the domain is invalid" do
+        stub_const("PotatoMesh::Application::INSTANCE_DOMAIN_SOURCE", :environment) do
+          allowed, reason = application_class.self_instance_registration_decision(nil)
+
+          expect(allowed).to be(false)
+          expect(reason).to eq("INSTANCE_DOMAIN missing or invalid")
+        end
+      end
+
+      it "rejects registration when the domain resolves to a restricted IP" do
+        stub_const("PotatoMesh::Application::INSTANCE_DOMAIN_SOURCE", :environment) do
+          allowed, reason = application_class.self_instance_registration_decision("127.0.0.1")
+
+          expect(allowed).to be(false)
+          expect(reason).to eq("INSTANCE_DOMAIN resolves to restricted IP")
+        end
+      end
+
+      it "accepts registration when configuration is valid" do
+        stub_const("PotatoMesh::Application::INSTANCE_DOMAIN_SOURCE", :environment) do
+          allowed, reason = application_class.self_instance_registration_decision(domain)
+
+          expect(allowed).to be(true)
+          expect(reason).to be_nil
+        end
+      end
+    end
+
+    describe ".ensure_self_instance_record!" do
+      it "persists the self instance when registration is allowed" do
+        stub_const("PotatoMesh::Application::INSTANCE_DOMAIN_SOURCE", :environment) do
+          stub_const("PotatoMesh::Application::INSTANCE_DOMAIN", "self.mesh") do
+            with_db do |db|
+              db.execute("DELETE FROM instances")
+            end
+
+            application_class.ensure_self_instance_record!
+
+            expect(instance_count).to eq(1)
+          end
+        end
+      end
+
+      it "skips persistence when registration is not allowed" do
+        stub_const("PotatoMesh::Application::INSTANCE_DOMAIN_SOURCE", :reverse_dns) do
+          with_db do |db|
+            db.execute("DELETE FROM instances")
+          end
+
+          application_class.ensure_self_instance_record!
+
+          expect(instance_count).to eq(0)
+        end
+      end
+    end
+  end
+
+  describe ".federation_target_domains" do
+    it "prioritises seed domains before database records" do
+      with_db do |db|
+        db.execute(
+          "INSERT INTO instances (id, domain, pubkey, name, version, channel, frequency, latitude, longitude, last_update_time, is_private, signature) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
+          [
+            "remote-id",
+            "Remote.Mesh",
+            "pubkey",
+            "Remote",
+            "1.0.0",
+            nil,
+            nil,
+            nil,
+            nil,
+            Time.now.to_i,
+            0,
+            "signature",
+          ],
+        )
+      end
+
+      targets = application_class.federation_target_domains("self.mesh")
+
+      expect(targets.first).to eq("potatomesh.net")
+      expect(targets).to include("remote.mesh")
+      expect(targets).not_to include("self.mesh")
+    end
+
+    it "falls back to seeds when the database is unavailable" do
+      allow(application_class).to receive(:open_database).and_raise(SQLite3::Exception.new("boom"))
+
+      targets = application_class.federation_target_domains("self.mesh")
+
+      expect(targets).to eq(["potatomesh.net"])
+    end
+  end
+
+  describe ".latest_node_update_timestamp" do
+    it "returns the maximum last_heard value" do
+      with_db do |db|
+        db.execute("DELETE FROM nodes")
+        db.execute("INSERT INTO nodes (node_id, last_heard) VALUES (?, ?)", ["node-a", 100])
+        db.execute("INSERT INTO nodes (node_id, last_heard) VALUES (?, ?)", ["node-b", 200])
+      end
+
+      expect(application_class.latest_node_update_timestamp).to eq(200)
+    end
+
+    it "returns nil when no nodes contain last_heard values" do
+      with_db do |db|
+        db.execute("DELETE FROM nodes")
+      end
+
+      expect(application_class.latest_node_update_timestamp).to be_nil
+    end
+  end
+
+  describe ".build_well_known_document" do
+    it "signs the payload and normalises the domain" do
+      with_db do |db|
+        db.execute("DELETE FROM nodes")
+        db.execute("INSERT INTO nodes (node_id, last_heard) VALUES (?, ?)", ["node-z", 321])
+      end
+
+      stub_const("PotatoMesh::Application::INSTANCE_DOMAIN", "Example.NET") do
+        json_output, signature = application_class.build_well_known_document
+        document = JSON.parse(json_output)
+
+        expect(document["domain"]).to eq("example.net")
+        expect(document["lastUpdate"]).to eq(321)
+        expect(document["signatureAlgorithm"]).to eq("rsa-sha256")
+        expect(signature).to be_a(String)
+        expect(signature).not_to be_empty
+      end
+    end
+  end
+
+  describe ".upsert_instance_record" do
+    it "rejects restricted domains" do
+      attributes = {
+        id: "restricted",
+        domain: "127.0.0.1",
+        pubkey: application_class::INSTANCE_PUBLIC_KEY_PEM,
+        name: nil,
+        version: nil,
+        channel: nil,
+        frequency: nil,
+        latitude: nil,
+        longitude: nil,
+        last_update_time: Time.now.to_i,
+        is_private: false,
+      }
+
+      expect do
+        with_db do |db|
+          application_class.upsert_instance_record(db, attributes, "sig")
+        end
+      end.to raise_error(ArgumentError, "restricted domain")
+    end
   end
 
   describe "logging configuration" do
@@ -1088,6 +1267,113 @@ RSpec.describe "Potato Mesh Sinatra app" do
         expect(row["is_private"]).to eq(0)
       end
     end
+
+    it "replaces an existing record when the domain is reused" do
+      with_db do |db|
+        db.execute(
+          <<~SQL,
+          INSERT INTO instances (
+            id, domain, pubkey, name, version, channel, frequency,
+            latitude, longitude, last_update_time, is_private, signature
+          ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+        SQL
+          [
+            "legacy-id",
+            domain,
+            "legacy-pubkey",
+            "Legacy Instance",
+            "0.9.0",
+            nil,
+            nil,
+            nil,
+            nil,
+            last_update_time - 100,
+            0,
+            "legacy-signature",
+          ],
+        )
+      end
+
+      debug_calls = []
+      allow_any_instance_of(Sinatra::Application).to receive(:debug_log).and_wrap_original do |method, *args, **kwargs|
+        debug_calls << [args, kwargs]
+        method.call(*args, **kwargs)
+      end
+
+      post "/api/instances", instance_payload.to_json, { "CONTENT_TYPE" => "application/json" }
+
+      expect(last_response.status).to eq(201)
+
+      with_db(readonly: true) do |db|
+        ids = db.execute("SELECT id FROM instances WHERE domain = ?", [domain]).flatten
+
+        expect(ids).to eq([instance_attributes[:id]])
+      end
+
+      expect(debug_calls).to include(
+        [
+          ["Removed conflicting instance by domain"],
+          hash_including(
+            context: "federation.instances",
+            domain: domain,
+            replaced_id: "legacy-id",
+            incoming_id: instance_attributes[:id],
+          ),
+        ],
+      )
+    end
+
+    it "normalises stored domains to lowercase" do
+      uppercase_payload = instance_payload.merge("domain" => "Mesh.Example")
+
+      post "/api/instances", uppercase_payload.to_json, { "CONTENT_TYPE" => "application/json" }
+
+      expect(last_response.status).to eq(201)
+
+      with_db(readonly: true) do |db|
+        stored = db.get_first_value("SELECT domain FROM instances WHERE id = ?", [instance_attributes[:id]])
+        expect(stored).to eq(domain)
+      end
+    end
+
+    it "rejects registrations missing last_heard data" do
+      missing_nodes = Array.new(PotatoMesh::Config.remote_instance_min_node_count) do |index|
+        { "node_id" => "remote-#{index}", "first_heard" => Time.now.to_i - index }
+      end
+
+      allow_any_instance_of(Sinatra::Application).to receive(:fetch_instance_json) do |_instance, host, path|
+        case path
+        when "/.well-known/potato-mesh"
+          [well_known_document, URI("https://#{host}#{path}")]
+        when "/api/nodes"
+          [missing_nodes, URI("https://#{host}#{path}")]
+        else
+          [nil, []]
+        end
+      end
+
+      warning_calls = []
+      allow_any_instance_of(Sinatra::Application).to receive(:warn_log).and_wrap_original do |method, *args, **kwargs|
+        warning_calls << [args, kwargs]
+        method.call(*args, **kwargs)
+      end
+
+      post "/api/instances", instance_payload.to_json, { "CONTENT_TYPE" => "application/json" }
+
+      expect(last_response.status).to eq(400)
+      expect(JSON.parse(last_response.body)).to eq("error" => "missing last_heard data")
+
+      expect(warning_calls).to include(
+        [
+          ["Instance registration rejected"],
+          hash_including(
+            context: "ingest.register",
+            domain: domain,
+            reason: "missing last_heard data",
+          ),
+        ],
+      )
+    end
   end
 
   describe "GET /api/instances" do

web/spec/sanitizer_spec.rb

@@ -35,9 +35,13 @@ RSpec.describe PotatoMesh::Sanitizer do
     end
 
     it "normalises valid domains" do
-      expect(described_class.sanitize_instance_domain(" Example.Org. ")).to eq("Example.Org")
+      expect(described_class.sanitize_instance_domain(" Example.Org. ")).to eq("example.org")
       expect(described_class.sanitize_instance_domain("[::1]")).to eq("[::1]")
     end
+
+    it "preserves case when requested" do
+      expect(described_class.sanitize_instance_domain("Mesh.Example", downcase: false)).to eq("Mesh.Example")
+    end
   end
 
   describe ".instance_domain_host" do

web/spec/spec_helper.rb

@@ -34,6 +34,7 @@ require "tmpdir"
 require "fileutils"
 
 ENV["RACK_ENV"] = "test"
+ENV["INSTANCE_DOMAIN"] ||= "spec.mesh.test"
 
 SPEC_TMPDIR = Dir.mktmpdir("potato-mesh-spec-")
 ENV["XDG_DATA_HOME"] = File.join(SPEC_TMPDIR, "xdg-data")