From ac2558f5ce6dca0f6f71823eea395283a107900b Mon Sep 17 00:00:00 2001 From: Benjamin ter Kuile Date: Tue, 22 Sep 2026 19:13:03 -0500 Subject: [PATCH] Fix critical webhook signature case bug; add correlationId + AR type registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strike sends X-Webhook-Signature in UPPERCASE hex. OpenSSL::HMAC. hexdigest always returns lowercase, and valid_signature? compared them case-sensitively via secure_compare — every genuine Strike webhook was being silently rejected as an invalid signature. Confirmed live: executed a real Lightning payment (payment-quotes/lightning + PATCH .../execute), received a real payment.updated webhook, and recomputing the HMAC over the captured payload/secret matched case-insensitively but not case-sensitively — proving the algorithm/secret/body were already correct and this was purely a case-comparison bug. Their own reference docs (docs.strike.me/webhooks/signature-verification) don't mention the case difference. Regression test added using the real captured signature, upcased. Also, both found via that same live test: - Invoices::Invoice didn't declare correlationId, raising ActiveModel::UnknownAttributeError the moment a real one (which CreateInvoice always sends) round-tripped through a live response — no existing spec fixture's stubbed body had included it. - BtcAttributeType was only registered with ActiveModel::Type, not ActiveRecord::Type (a separate registry) — an `attribute :x, :strike_btc` declaration on an ActiveRecord model raised "Unknown type" without this. Guarded behind defined?(ActiveRecord::Type) since this gem only depends on active_model. Co-Authored-By: Claude Sonnet 5 --- lib/strike_me/api/invoices/invoice.rb | 9 +++++++ lib/strike_me/api/webhook.rb | 13 +++++++++- lib/strike_me/btc_attribute_type.rb | 9 +++++++ spec/strike_me/api/invoices_spec.rb | 34 ++++++++++++++------------- spec/strike_me/api/webhook_spec.rb | 11 +++++++++ 5 files changed, 59 insertions(+), 17 deletions(-) diff --git a/lib/strike_me/api/invoices/invoice.rb b/lib/strike_me/api/invoices/invoice.rb index cf43104..4a4d35e 100644 --- a/lib/strike_me/api/invoices/invoice.rb +++ b/lib/strike_me/api/invoices/invoice.rb @@ -12,6 +12,15 @@ module StrikeMe attribute :issuerId attribute :receiverId attribute :payerId + # Only present on the #create response, echoing back whatever + # Invoices#create sent as correlationId (Strike's own docs list + # it as request-optional/response-present) — missing here meant + # ActiveModel::UnknownAttributeError the first time a real + # correlation_id round-tripped through a live Strike account (no + # existing spec fixture's stubbed response body included it, so + # this went uncaught until now). end_with?('Id') below already + # picks this up automatically, same as every other *Id field. + attribute :correlationId alias_attribute :id, :invoiceId attribute_names.select { _1.end_with? 'Id' }.each do |snake_name| diff --git a/lib/strike_me/api/webhook.rb b/lib/strike_me/api/webhook.rb index 3a3743e..6dc3e95 100644 --- a/lib/strike_me/api/webhook.rb +++ b/lib/strike_me/api/webhook.rb @@ -22,7 +22,18 @@ module StrikeMe return false if signature.blank? || raw_body.blank? expected = OpenSSL::HMAC.hexdigest('SHA256', secret, raw_body) - secure_compare(expected, signature) + # Strike sends X-Webhook-Signature in UPPERCASE hex — confirmed + # live against a real webhook delivery (payment.updated, a real + # executed Lightning send) 2026-09-22. OpenSSL::HMAC.hexdigest + # always returns lowercase. Without downcasing both sides here, + # secure_compare's case-sensitive byte comparison rejected every + # genuine Strike webhook as an invalid signature — silently: a + # rejected webhook looks identical to an actual forgery attempt + # from the outside, so this would have broken settlement in + # production with no visible error at all. Their own docs' + # reference implementation (docs.strike.me/webhooks/ + # signature-verification) doesn't mention this case difference. + secure_compare(expected.downcase, signature.downcase) end # ActiveSupport::SecurityUtils.secure_compare raises on a length diff --git a/lib/strike_me/btc_attribute_type.rb b/lib/strike_me/btc_attribute_type.rb index 93bbd90..04ee83f 100644 --- a/lib/strike_me/btc_attribute_type.rb +++ b/lib/strike_me/btc_attribute_type.rb @@ -17,3 +17,12 @@ module StrikeMe end ActiveModel::Type.register(:strike_btc, StrikeMe::BtcAttributeType) +# ActiveRecord keeps its own type registry (ActiveRecord::Type:: +# AdapterSpecificRegistry), separate from ActiveModel::Type.registry +# above — an `attribute :amount, :strike_btc` declaration on an +# ActiveRecord model looks it up there, not here, and raises "Unknown +# type :strike_btc" without this second registration (confirmed live). +# Guarded: this gem itself only depends on active_model, not +# active_record — a plain ActiveModel::Model consumer never loads +# ActiveRecord at all, and this must not be what breaks that. +ActiveRecord::Type.register(:strike_btc, StrikeMe::BtcAttributeType) if defined?(ActiveRecord::Type) diff --git a/spec/strike_me/api/invoices_spec.rb b/spec/strike_me/api/invoices_spec.rb index f6913ba..ccb2dba 100644 --- a/spec/strike_me/api/invoices_spec.rb +++ b/spec/strike_me/api/invoices_spec.rb @@ -13,27 +13,29 @@ RSpec.describe StrikeMe::API::Invoices do expect(list.last ).not_to be_paid expect(list.first.attributes).to eq( - 'invoiceId' => '97dca64c-6dde-4a87-b970-279f1a8e0ac5', - 'amount' => {'amount' => '0.00014', 'currency' => 'BTC'}, - 'state' => 'PAID', - 'created' => '2025-08-08 22:28:44.195825 +0000', # is actual Time - 'description' => 'Hi there', - 'issuerId' => '41952e07-483c-4cd7-afa4-b6a8ec729809', - 'receiverId' => '41952e07-483c-4cd7-afa4-b6a8ec729809', - 'payerId' => 'f75c22bf-625f-4c47-acdf-acb9c596bb8c' + 'invoiceId' => '97dca64c-6dde-4a87-b970-279f1a8e0ac5', + 'amount' => {'amount' => '0.00014', 'currency' => 'BTC'}, + 'state' => 'PAID', + 'created' => '2025-08-08 22:28:44.195825 +0000', # is actual Time + 'description' => 'Hi there', + 'issuerId' => '41952e07-483c-4cd7-afa4-b6a8ec729809', + 'receiverId' => '41952e07-483c-4cd7-afa4-b6a8ec729809', + 'payerId' => 'f75c22bf-625f-4c47-acdf-acb9c596bb8c', + 'correlationId' => nil ) expect(list.first.id ).to eq '97dca64c-6dde-4a87-b970-279f1a8e0ac5' expect(list.first.created).to be_a Time expect(list.last.attributes).to eq( - 'invoiceId' => 'f3853f94-d198-4b83-8b9a-11a836926819', - 'amount' => {'amount' => '40.00', 'currency' => 'USDT'}, - 'state' => 'UNPAID', - 'created' => '2023-05-20 22:44:36.856775 +0000', - 'description' => '', - 'issuerId' => 'f75c22bf-625f-4c47-acdf-acb9c596bb8c', - 'receiverId' => 'f75c22bf-625f-4c47-acdf-acb9c596bb8c', - 'payerId' => nil + 'invoiceId' => 'f3853f94-d198-4b83-8b9a-11a836926819', + 'amount' => {'amount' => '40.00', 'currency' => 'USDT'}, + 'state' => 'UNPAID', + 'created' => '2023-05-20 22:44:36.856775 +0000', + 'description' => '', + 'issuerId' => 'f75c22bf-625f-4c47-acdf-acb9c596bb8c', + 'receiverId' => 'f75c22bf-625f-4c47-acdf-acb9c596bb8c', + 'correlationId' => nil, + 'payerId' => nil ) end end diff --git a/spec/strike_me/api/webhook_spec.rb b/spec/strike_me/api/webhook_spec.rb index 06c0cc3..087aa7f 100644 --- a/spec/strike_me/api/webhook_spec.rb +++ b/spec/strike_me/api/webhook_spec.rb @@ -10,6 +10,17 @@ RSpec.describe StrikeMe::API::Webhook do expect(described_class.valid_signature?(raw_body: raw_body, signature: real_signature, secret: secret)).to eq true end + # Strike sends X-Webhook-Signature in UPPERCASE hex — confirmed live + # against a real payment.updated delivery 2026-09-22 (executed a real + # Lightning send, received a real signed webhook). OpenSSL::HMAC. + # hexdigest always returns lowercase; without downcasing both sides + # this rejected every genuine Strike webhook, silently, since a + # rejected webhook is indistinguishable from an actual forgery + # attempt from outside. + it 'is true for the real signature even when Strike sends it as UPPERCASE hex' do + expect(described_class.valid_signature?(raw_body: raw_body, signature: real_signature.upcase, secret: secret)).to eq true + end + it 'is false for the wrong secret' do expect(described_class.valid_signature?(raw_body: raw_body, signature: real_signature, secret: 'wrong')).to eq false end