Skip to content

Commit a254ee8

Browse files
dblockCopilot
andcommitted
Support non-rewindable Rack inputs for Events signature verification
Follow up on #515 with signature and WEBrick handler integration coverage for Rack 2 and Rack 3, plus middleware ordering documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 565c386 commit a254ee8

8 files changed

Lines changed: 155 additions & 4 deletions

File tree

‎.github/workflows/test.yml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,13 @@ jobs:
1111
- { ruby: "3.1" }
1212
- { ruby: "3.2" }
1313
- { ruby: "3.3" }
14+
- { ruby: "3.3", rackup: "~> 1.0" }
1415
- { ruby: "3.4" }
1516
- { ruby: ruby-head, ignore: true }
1617
- { ruby: jruby-head, ignore: true }
17-
name: test (ruby=${{ matrix.entry.ruby }})
18+
name: test (ruby=${{ matrix.entry.ruby }}, rackup=${{ matrix.entry.rackup || '~> 2.1' }})
19+
env:
20+
RACKUP_VERSION: ${{ matrix.entry.rackup || '~> 2.1' }}
1821
steps:
1922
- name: Checkout
2023
uses: actions/checkout@v7
@@ -34,7 +37,7 @@ jobs:
3437
with:
3538
parallel: true
3639
github-token: ${{ secrets.GITHUB_TOKEN }}
37-
flag-name: run-${{ matrix.entry.ruby }}
40+
flag-name: run-${{ matrix.entry.ruby }}${{ matrix.entry.rackup && '-rack2' || '' }}
3841
allow-empty: ${{ matrix.entry.ignore || false }}
3942

4043
finish:

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
### 3.3.0 (Next)
22

3+
* Support non-rewindable Rack 3 request bodies when verifying Events API signatures, with Rack 2/3 and WEBrick coverage; follows [#515](https://github.com/slack-ruby/slack-ruby-client/pull/515) - [@olleolleolle](https://github.com/olleolleolle), [@dblock](https://github.com/dblock).
34
* [#606](https://github.com/slack-ruby/slack-ruby-client/pull/606): Add configured client copies with error callbacks for Slack and transport failures - [@dblock](https://github.com/dblock).
45
* [#605](https://github.com/slack-ruby/slack-ruby-client/pull/605): Add configured client copies with request and response callbacks, including pagination - [@dblock](https://github.com/dblock).
56
* [#603](https://github.com/slack-ruby/slack-ruby-client/pull/603): Enforce consistent exception messages with rubocop-exception_messages and run lint separately from the test matrix - [@dblock](https://github.com/dblock).

‎CONTRIBUTING.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,17 @@ bundle install
2525
bundle exec rake
2626
```
2727

28+
Events request integration specs exercise WEBrick's HTTP parser and Rack handler with both Rack 3's
29+
non-rewindable input and Rack 2's rewindable input. The default test bundle uses rackup 2 / Rack 3;
30+
CI also runs rackup 1 / Rack 2. To select the older combination locally:
31+
32+
```bash
33+
RACKUP_VERSION='~> 1.0' bundle update rackup rack
34+
RACKUP_VERSION='~> 1.0' bundle exec rspec spec/slack/events
35+
```
36+
37+
Unset `RACKUP_VERSION` and run `bundle update rackup rack` to restore the default combination.
38+
2839
### Run Examples in Development
2940

3041
Sign up for Slack, create a private Slack team for yourself, then [generate an API token](https://api.slack.com/tutorials/tracks/getting-a-token) for your app and use it for some interactions. To get a token you will need to install the app in a workspace, and use the "User OAuth Token" from installed app settings.

‎Gemfile‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ group :test do
1616
gem 'json-schema'
1717
gem 'mutex_m'
1818
gem 'racc'
19+
gem 'rackup', ENV.fetch('RACKUP_VERSION', '~> 2.1')
1920
gem 'rake', '~> 13'
2021
gem 'rspec'
2122
# Lock below 1.1.0, which started writing float timestamps to
@@ -26,6 +27,7 @@ group :test do
2627
gem 'timecop'
2728
gem 'vcr'
2829
gem 'webmock'
30+
gem 'webrick', '~> 1.8'
2931
end
3032

3133
if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.2')

‎README.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,16 @@ Slack::Events::Request.new(
461461

462462
The `verify!` call may raise `Slack::Events::Request::MissingSigningSecret`, `Slack::Events::Request::InvalidSignature` or `Slack::Events::Request::TimestampExpired` errors.
463463

464+
Both rewindable request bodies and non-rewindable streams permitted by Rack 3 are supported.
465+
`Slack::Events::Request#body` caches the raw body. Rewindable inputs are rewound before and after reading,
466+
preserving access for other consumers. Non-rewindable inputs are consumed once; use `slack_request.body`
467+
for subsequent access to the raw body.
468+
469+
Verify the signature before middleware or application code reads a non-rewindable input. Bytes already
470+
consumed from a one-shot stream cannot be recovered, and verification will fail. If multiple consumers
471+
need to read `rack.input`, install `Rack::RewindableInput::Middleware` (Rack 3) before any body-reading
472+
middleware to buffer the input.
473+
464474
### Message Handling
465475

466476
All text in Slack uses the same [system of formatting and escaping](https://api.slack.com/docs/formatting): chat messages, direct messages, file comments, etc. [Slack::Messages::Formatting](lib/slack/messages/formatting.rb) provides convenience methods to format and parse messages.

‎lib/slack/events/request.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ def version
4040
def body
4141
@body ||= begin
4242
input = http_request.body
43-
input.rewind
43+
input.rewind if input.respond_to?(:rewind)
4444
body = input.read
45-
input.rewind
45+
input.rewind if input.respond_to?(:rewind)
4646
body
4747
end
4848
end
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# frozen_string_literal: true
2+
require 'spec_helper'
3+
require 'rack'
4+
require 'rack/lint'
5+
require 'rack/rewindable_input'
6+
if Gem::Version.new(Rack.release) < Gem::Version.new('3')
7+
require 'rack/handler/webrick'
8+
else
9+
require 'rackup/handler/webrick'
10+
end
11+
12+
RSpec.describe Slack::Events::Request do
13+
let(:signing_secret) { 'test-signing-secret' }
14+
let(:timestamp) { Time.now.to_i.to_s }
15+
let(:body) { '{"type":"url_verification","challenge":"hello"}' }
16+
let(:signature) do
17+
"v0=#{OpenSSL::HMAC.hexdigest('SHA256', signing_secret, "v0:#{timestamp}:#{body}")}"
18+
end
19+
let(:consume_body) { false }
20+
let(:buffer_body) { false }
21+
let(:observed_inputs) { [] }
22+
let(:app) do
23+
Rack::Lint.new(lambda do |env|
24+
observed_inputs << env.fetch('rack.input')
25+
env['rack.input'] = Rack::RewindableInput.new(env['rack.input']) if buffer_body
26+
env['rack.input'].read if consume_body
27+
slack_request = described_class.new(Rack::Request.new(env), signing_secret: signing_secret)
28+
begin
29+
slack_request.verify!
30+
[200, { 'content-type' => 'application/json' }, [slack_request.body, slack_request.body]]
31+
rescue Slack::Events::Request::InvalidSignature
32+
[401, { 'content-type' => 'text/plain' }, ['invalid signature']]
33+
ensure
34+
env['rack.input'].close if buffer_body
35+
end
36+
end)
37+
end
38+
let(:server) { WEBrick::HTTPServer.new(DoNotListen: true, Logger: WEBrick::Log.new(File::NULL), AccessLog: []) }
39+
40+
def http_request
41+
raw_request = [
42+
'POST /events HTTP/1.1',
43+
'Host: localhost',
44+
'Content-Type: application/json',
45+
"Content-Length: #{body.bytesize}",
46+
"X-Slack-Request-Timestamp: #{timestamp}",
47+
"X-Slack-Signature: #{signature}",
48+
'', body
49+
].join("\r\n")
50+
WEBrick::HTTPRequest.new(server.config).tap { |request| request.parse(StringIO.new(raw_request)) }
51+
end
52+
53+
after do
54+
server.shutdown
55+
end
56+
57+
def dispatch
58+
response = WEBrick::HTTPResponse.new(server.config)
59+
handler = Gem::Version.new(Rack.release) < Gem::Version.new('3') ? Rack::Handler::WEBrick : Rackup::Handler::WEBrick
60+
handler.new(server, app).service(http_request, response)
61+
response
62+
end
63+
64+
it 'verifies a signed WEBrick request and caches its complete body' do
65+
http_response = dispatch
66+
expect(http_response.status).to eq 200
67+
expect(http_response.body).to eq body * 2
68+
expect(observed_inputs.first.respond_to?(:rewind)).to eq(Gem::Version.new(Rack.release) < Gem::Version.new('3'))
69+
end
70+
71+
context 'with an invalid signature' do
72+
let(:signature) { 'v0=invalid' }
73+
74+
it 'rejects the request' do
75+
http_response = dispatch
76+
expect(http_response.status).to eq 401
77+
end
78+
end
79+
80+
context 'with input already read by another consumer' do
81+
let(:consume_body) { true }
82+
83+
it 'only verifies the request if the server input can be rewound' do
84+
http_response = dispatch
85+
expected_status = Gem::Version.new(Rack.release) < Gem::Version.new('3') ? 200 : 401
86+
expect(http_response.status).to eq expected_status
87+
end
88+
89+
context 'with input buffering before the first consumer' do
90+
let(:buffer_body) { true }
91+
92+
it 'verifies the request and preserves the full body' do
93+
http_response = dispatch
94+
expect(http_response.status).to eq 200
95+
expect(http_response.body).to eq body * 2
96+
end
97+
end
98+
end
99+
end

‎spec/slack/events/request_spec.rb‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,31 @@
7070
end
7171
end
7272

73+
context 'with a non-rewindable body' do
74+
let(:input) { double(read: body) }
75+
76+
before do
77+
allow(http_request).to receive(:body).and_return(input)
78+
end
79+
80+
it 'reads and caches the body without rewinding' do
81+
expect(input).to receive(:read).once.and_return(body)
82+
2.times { expect(request.body).to eq body }
83+
end
84+
85+
it 'validates the signature' do
86+
expect(request).to be_valid
87+
end
88+
89+
context 'with an already consumed body' do
90+
let(:input) { double(read: '') }
91+
92+
it 'rejects the signature rather than accepting an incomplete body' do
93+
expect(request).not_to be_valid
94+
end
95+
end
96+
end
97+
7398
context 'time' do
7499
after do
75100
Timecop.return

0 commit comments

Comments
 (0)