Skip to content

Commit 9b41b1f

Browse files
nevyangelovaNevyana Angelova
andauthored
Limit webhook request body size with http.MaxBytesReader (#995)
Co-authored-by: Nevyana Angelova <nevyangelova@192.168.100.47>
1 parent c569eaf commit 9b41b1f

File tree

2 files changed

+56
-0
lines changed

2 files changed

+56
-0
lines changed

server/plugin/webhook.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"crypto/sha1" //nolint:gosec // GitHub webhooks are signed using sha1 https://developer.github.com/webhooks/.
1010
"encoding/hex"
1111
"encoding/json"
12+
"errors"
1213
"html"
1314
"io"
1415
"net/http"
@@ -199,11 +200,20 @@ func (wb *WebhookBroker) Close() {
199200
}
200201
}
201202

203+
const maxWebhookPayloadSize = 25 * 1024 * 1024 // 25 MB, matching GitHub's documented maximum
204+
202205
func (p *Plugin) handleWebhook(w http.ResponseWriter, r *http.Request) {
203206
p.client.Log.Info("Webhook event received")
204207
config := p.getConfiguration()
208+
209+
r.Body = http.MaxBytesReader(w, r.Body, maxWebhookPayloadSize)
205210
body, err := io.ReadAll(r.Body)
206211
if err != nil {
212+
var maxBytesErr *http.MaxBytesError
213+
if errors.As(err, &maxBytesErr) {
214+
http.Error(w, "Request body too large", http.StatusRequestEntityTooLarge)
215+
return
216+
}
207217
http.Error(w, "Bad request body", http.StatusBadRequest)
208218
return
209219
}

server/plugin/webhook_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,52 @@ const (
3030
gitHubOrginization = "test-org"
3131
)
3232

33+
func TestHandleWebhookBodySizeLimit(t *testing.T) {
34+
t.Run("rejects oversized request body", func(t *testing.T) {
35+
_, mockAPI, _, _, _ := GetTestSetup(t)
36+
p := NewPlugin()
37+
p.initializeAPI()
38+
p.SetAPI(mockAPI)
39+
p.client = pluginapi.NewClient(mockAPI, p.Driver)
40+
p.setConfiguration(&Configuration{
41+
WebhookSecret: webhookSecret,
42+
})
43+
44+
mockAPI.On("LogInfo", "Webhook event received")
45+
46+
oversizedBody := strings.NewReader(strings.Repeat("x", 26*1024*1024))
47+
req := httptest.NewRequest(http.MethodPost, "/webhook", oversizedBody)
48+
req.Header.Set("X-Hub-Signature", "sha1=invalid")
49+
w := httptest.NewRecorder()
50+
51+
p.handleWebhook(w, req)
52+
53+
assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code)
54+
})
55+
56+
t.Run("accepts normal sized request body", func(t *testing.T) {
57+
_, mockAPI, _, _, _ := GetTestSetup(t)
58+
p := NewPlugin()
59+
p.initializeAPI()
60+
p.SetAPI(mockAPI)
61+
p.client = pluginapi.NewClient(mockAPI, p.Driver)
62+
p.setConfiguration(&Configuration{
63+
WebhookSecret: webhookSecret,
64+
})
65+
66+
mockAPI.On("LogInfo", "Webhook event received")
67+
68+
body := `{"zen": "test"}`
69+
req := httptest.NewRequest(http.MethodPost, "/webhook", strings.NewReader(body))
70+
req.Header.Set("X-Hub-Signature", "sha1=invalid")
71+
w := httptest.NewRecorder()
72+
73+
p.handleWebhook(w, req)
74+
75+
assert.Equal(t, http.StatusUnauthorized, w.Code)
76+
})
77+
}
78+
3379
func TestPostPushEvent(t *testing.T) {
3480
tests := []struct {
3581
name string

0 commit comments

Comments
 (0)