fix(message-db): prevent SQL injection via condition keys (GHSA-x2hq-rfpg-3xr5)

The Legacy API message lookup endpoints parsed the request body as JSON and
passed the `id` parameter straight through to the message database. A JSON
object supplied for `id` arrived as a Ruby Hash and was used as a raw set of
SQL `WHERE` conditions. `hash_to_sql` interpolated each Hash key directly
inside backtick identifier quoting while escaping only the value, so a key
containing a backtick could break out of the identifier and inject arbitrary
SQL into the SELECT (blind, time-based) against the message database.

Fixes:

- Escape all identifiers (columns, tables, database names) through a new
  `escape_identifier` helper that wraps in backticks and doubles embedded
  backticks. Applied across hash_to_sql, select, insert, insert_multi,
  update and delete so no caller can inject via an identifier.
- Validate the Legacy API `id` parameter at the controller boundary: reject
  any non-scalar value before it reaches the database and coerce it to an
  integer. Internal Hash-based lookups (e.g. tracking middleware) are
  unaffected.

Adds regression tests at the unit (hash_to_sql / escape_identifier) and
request (legacy messages/deliveries) levels.

spec/apis/legacy_api/messages/deliveries_spec.rb

@@ -64,6 +64,23 @@ RSpec.describe "Legacy Messages API", type: :request do
         end
       end
 
+      # Regression test for GHSA-x2hq-rfpg-3xr5 (see message_spec.rb). A JSON
+      # object supplied for `id` must be rejected before reaching the database
+      # rather than being interpreted as a raw set of SQL conditions.
+      context "when the message ID is a JSON object (SQL injection attempt)" do
+        it "rejects it with a parameter error and never reaches the database" do
+          expect_any_instance_of(Server).not_to receive(:message)
+          post "/api/v1/messages/deliveries",
+               headers: { "x-server-api-key" => credential.key,
+                          "content-type" => "application/json" },
+               params: { id: { "id`=0 OR SLEEP(5)#" => "x" } }.to_json
+          expect(response.status).to eq 200
+          parsed_body = JSON.parse(response.body)
+          expect(parsed_body["status"]).to eq "parameter-error"
+          expect(parsed_body["data"]["message"]).to match(/must be a string or integer/)
+        end
+      end
+
       context "when the message ID exists" do
         let(:server) { create(:server) }
         let(:credential) { create(:credential, server: server) }

spec/apis/legacy_api/messages/message_spec.rb

@@ -63,6 +63,56 @@ RSpec.describe "Legacy Messages API", type: :request do
         end
       end
 
+      # Regression tests for GHSA-x2hq-rfpg-3xr5. The request body is parsed as
+      # JSON, so a JSON object/array supplied for `id` would otherwise arrive as
+      # a Ruby Hash/Array and be passed straight through to the message database
+      # as a raw set of SQL conditions (blind SQL injection). These must be
+      # rejected before reaching the database.
+      context "when the message ID is a JSON object (SQL injection attempt)" do
+        it "rejects it with a parameter error and never reaches the database" do
+          expect_any_instance_of(Server).not_to receive(:message)
+          post "/api/v1/messages/message",
+               headers: { "x-server-api-key" => credential.key,
+                          "content-type" => "application/json" },
+               params: { id: { "id`=0 OR SLEEP(5)#" => "x" } }.to_json
+          expect(response.status).to eq 200
+          parsed_body = JSON.parse(response.body)
+          expect(parsed_body["status"]).to eq "parameter-error"
+          expect(parsed_body["data"]["message"]).to match(/must be a string or integer/)
+        end
+      end
+
+      context "when the message ID is a JSON array" do
+        it "rejects it with a parameter error" do
+          post "/api/v1/messages/message",
+               headers: { "x-server-api-key" => credential.key,
+                          "content-type" => "application/json" },
+               params: { id: [1, 2, 3] }.to_json
+          expect(response.status).to eq 200
+          parsed_body = JSON.parse(response.body)
+          expect(parsed_body["status"]).to eq "parameter-error"
+          expect(parsed_body["data"]["message"]).to match(/must be a string or integer/)
+        end
+      end
+
+      context "when the message ID is provided as a numeric string" do
+        let(:message) { MessageFactory.outgoing(server) }
+
+        it "is coerced to an integer and looks the message up" do
+          post "/api/v1/messages/message",
+               headers: { "x-server-api-key" => credential.key,
+                          "content-type" => "application/json" },
+               params: { id: message.id.to_s }.to_json
+          expect(response.status).to eq 200
+          parsed_body = JSON.parse(response.body)
+          expect(parsed_body["status"]).to eq "success"
+          expect(parsed_body["data"]).to match({
+            "id" => message.id,
+            "token" => message.token
+          })
+        end
+      end
+
       context "when the message ID exists" do
         let(:server) { create(:server) }
         let(:credential) { create(:credential, server: server) }

spec/lib/postal/message_db/database_spec.rb

@@ -14,5 +14,64 @@ describe Postal::MessageDB::Database do
     it "should return the current schema version" do
       expect(database.schema_version).to be_a Integer
     end
+
+    describe "#escape_identifier" do
+      it "wraps a plain identifier in backticks" do
+        expect(database.send(:escape_identifier, "id")).to eq "`id`"
+      end
+
+      it "doubles embedded backticks so the value cannot break out of the quoting" do
+        expect(database.send(:escape_identifier, "id`=0 OR SLEEP(5)#"))
+          .to eq "`id``=0 OR SLEEP(5)#`"
+      end
+
+      it "coerces non-string identifiers to a string" do
+        expect(database.send(:escape_identifier, :token)).to eq "`token`"
+      end
+    end
+
+    describe "#hash_to_sql" do
+      it "builds a simple equality condition" do
+        expect(database.send(:hash_to_sql, { "id" => 5 })).to eq "`id` = '5'"
+      end
+
+      it "builds an IN condition for an array of integers" do
+        expect(database.send(:hash_to_sql, { "id" => [1, 2] })).to eq "`id` IN (1, 2)"
+      end
+
+      it "builds operator conditions for a hash value" do
+        expect(database.send(:hash_to_sql, { "id" => { greater_than: 1 } }))
+          .to eq "`id` > '1'"
+      end
+
+      # Regression tests for GHSA-x2hq-rfpg-3xr5: a backtick in the condition
+      # key must be neutralised so it cannot close the identifier quoting and
+      # inject arbitrary SQL.
+      it "neutralises a backtick injection in an equality key" do
+        sql = database.send(:hash_to_sql, { "id`=0 OR SLEEP(5)#" => "x" })
+        expect(sql).to eq "`id``=0 OR SLEEP(5)#` = 'x'"
+      end
+
+      it "neutralises a backtick injection in an IN key" do
+        sql = database.send(:hash_to_sql, { "id`)#" => %w[a b] })
+        expect(sql).to eq "`id``)#` IN ('a', 'b')"
+      end
+
+      it "neutralises a backtick injection in an operator key" do
+        sql = database.send(:hash_to_sql, { "id`#" => { greater_than: 1 } })
+        expect(sql).to eq "`id``#` > '1'"
+      end
+    end
+
+    describe "#select with a hostile condition key" do
+      # End-to-end proof against the live test database: the injected key is
+      # treated as a single (non-existent) column identifier, so MySQL rejects
+      # the query instead of executing the injected SQL.
+      it "does not allow SQL injection via the condition key" do
+        expect do
+          database.select("messages", where: { "id`=0 OR 1=1#" => "x" }, limit: 1)
+        end.to raise_error(Mysql2::Error)
+      end
+    end
   end
 end