tiennm99/goclaw
1
0 Fork 0
mirror of https://github.com/tiennm99/goclaw.git synced 2026-10-19 18:15:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): harden upstream critical surfaces (#32)

Browse Source 
* fix(security): harden upstream critical surfaces

Refs #30

* fix(security): close pre-landing review gaps

Refs #30

* fix(security): close official release blockers
This commit is contained in:
Duy /zuey/ authored and GitHub committed 2026-05-20 16:33:49 +07:00
1 parent 2a48e62c6b
commit 532ff91d8e
59 files changed
+2194 -397
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/channels/feishu/larkevents.go
+35 -10
View File
@@ -13,6 +13,8 @@ import (
"strings"
)
const maxWebhookBodyBytes = 1 << 20
// --- Event types (replacing larkim.P2MessageReceiveV1) ---
// MessageEvent is the parsed structure of a Feishu im.message.receive_v1 event.
@@ -42,9 +44,9 @@ type EventSender struct {
}
type EventMessage struct {
MessageID string `json:"message_id"`
RootID string `json:"root_id"`
ParentID string `json:"parent_id"`
MessageID string `json:"message_id"`
RootID string `json:"root_id"`
ParentID string `json:"parent_id"`
// ThreadID is the definitive "this message lives inside a thread" signal
// per Lark docs. Unlike RootID (which is populated on ANY reply — including
// plain quote replies), ThreadID is only present when the message is in an
@@ -59,8 +61,8 @@ type EventMessage struct {
}
type EventMention struct {
Key string `json:"key"`
ID struct {
Key string `json:"key"`
ID struct {
OpenID string `json:"open_id"`
UserID string `json:"user_id"`
UnionID string `json:"union_id"`
@@ -75,9 +77,9 @@ type EventMention struct {
// Schema v1.0 uses flat structure, v2.0 uses header+event.
type webhookEvent struct {
// v2.0 fields
Schema string `json:"schema"`
Header json.RawMessage `json:"header"`
Event json.RawMessage `json:"event"`
Schema string `json:"schema"`
Header json.RawMessage `json:"header"`
Event json.RawMessage `json:"event"`
// v1.0 fields (also used for URL verification challenge)
Type string `json:"type"`
@@ -97,11 +99,15 @@ func NewWebhookHandler(verificationToken, encryptKey string, onMessage func(even
return
}
body, err := io.ReadAll(r.Body)
body, err := io.ReadAll(io.LimitReader(r.Body, maxWebhookBodyBytes+1))
if err != nil {
http.Error(w, "read body failed", http.StatusBadRequest)
return
}
if len(body) > maxWebhookBodyBytes {
http.Error(w, "body too large", http.StatusRequestEntityTooLarge)
return
}
// Try to decrypt if encrypted
var envelope webhookEvent
@@ -129,11 +135,22 @@ func NewWebhookHandler(verificationToken, encryptKey string, onMessage func(even
// URL verification challenge
if envelope.Type == "url_verification" {
if verificationToken == "" || envelope.Token != verificationToken {
slog.Warn("security.feishu_webhook_url_verification_rejected")
w.WriteHeader(http.StatusOK)
return
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]string{"challenge": envelope.Challenge})
return
}
if encryptKey != "" && envelope.Encrypt == "" {
slog.Warn("security.feishu_webhook_plaintext_rejected")
w.WriteHeader(http.StatusOK)
return
}
// Parse as message event
var event MessageEvent
@@ -144,8 +161,13 @@ func NewWebhookHandler(verificationToken, encryptKey string, onMessage func(even
}
// Verify token if configured
if verificationToken == "" && encryptKey == "" {
slog.Warn("security.feishu_webhook_missing_verification")
w.WriteHeader(http.StatusOK)
return
}
if verificationToken != "" && event.Header.Token != verificationToken {
slog.Warn("feishu webhook token mismatch")
slog.Warn("security.feishu_webhook_token_mismatch")
w.WriteHeader(http.StatusOK)
return
}
@@ -181,6 +203,9 @@ func decryptEvent(encryptedBase64, key string) ([]byte, error) {
// IV is first 16 bytes
iv := ciphertext[:aes.BlockSize]
ciphertext = ciphertext[aes.BlockSize:]
if len(ciphertext) == 0 || len(ciphertext)%aes.BlockSize != 0 {
return nil, fmt.Errorf("ciphertext length not block aligned")
}
mode := cipher.NewCBCDecrypter(block, iv)
mode.CryptBlocks(ciphertext, ciphertext)
internal/channels/feishu/larkevents_test.go
+104 -1
View File
@@ -63,7 +63,7 @@ func buildWebhookRequest(body string) *http.Request {
func TestWebhookHandler_URLVerification(t *testing.T) {
called := false
h := NewWebhookHandler("", "", func(_ *MessageEvent) { called = true })
h := NewWebhookHandler("test-tok", "", func(_ *MessageEvent) { called = true })
body := `{"type":"url_verification","token":"test-tok","challenge":"abc123"}`
w := httptest.NewRecorder()
@@ -84,6 +84,24 @@ func TestWebhookHandler_URLVerification(t *testing.T) {
}
}
func TestWebhookHandler_URLVerificationRequiresMatchingToken(t *testing.T) {
h := NewWebhookHandler("expected-token", "", func(_ *MessageEvent) {
t.Fatal("onMessage must not be called for url_verification")
})
body := `{"type":"url_verification","token":"wrong-token","challenge":"abc123"}`
w := httptest.NewRecorder()
h.ServeHTTP(w, buildWebhookRequest(body))
if w.Code != http.StatusOK {
t.Errorf("status: got %d, want 200", w.Code)
}
var resp map[string]string
if err := json.NewDecoder(w.Body).Decode(&resp); err == nil && resp["challenge"] != "" {
t.Fatalf("must not return challenge for mismatched token, got %q", resp["challenge"])
}
}
// --- Method not allowed ---
func TestWebhookHandler_MethodNotAllowed(t *testing.T) {
@@ -179,6 +197,72 @@ func TestWebhookHandler_TokenMatch_Dispatches(t *testing.T) {
}
}
func TestWebhookHandler_MissingVerificationTokenDoesNotDispatchMessage(t *testing.T) {
dispatched := make(chan *MessageEvent, 1)
h := NewWebhookHandler("", "", func(e *MessageEvent) { dispatched <- e })
env := map[string]any{
"schema": "2.0",
"header": map[string]any{
"event_id": "evt_missing_token",
"event_type": "im.message.receive_v1",
"token": "",
"app_id": "cli_test",
"tenant_key": "test-tenant-1",
},
"event": map[string]any{
"sender": map[string]any{},
"message": map[string]any{"message_id": "om_1", "chat_id": "oc_1"},
},
}
body, _ := json.Marshal(env)
w := httptest.NewRecorder()
h.ServeHTTP(w, buildWebhookRequest(string(body)))
if w.Code != http.StatusOK {
t.Errorf("status: got %d, want 200", w.Code)
}
select {
case <-dispatched:
t.Fatal("onMessage must not be called when verification token is missing")
case <-time.After(100 * time.Millisecond):
}
}
func TestWebhookHandler_EncryptKeyRejectsPlaintextEvent(t *testing.T) {
dispatched := make(chan *MessageEvent, 1)
h := NewWebhookHandler("", "encrypt-key", func(e *MessageEvent) { dispatched <- e })
env := map[string]any{
"schema": "2.0",
"header": map[string]any{
"event_id": "evt_plaintext",
"event_type": "im.message.receive_v1",
"token": "",
"app_id": "cli_test",
"tenant_key": "test-tenant-1",
},
"event": map[string]any{
"sender": map[string]any{},
"message": map[string]any{"message_id": "om_1", "chat_id": "oc_1"},
},
}
body, _ := json.Marshal(env)
w := httptest.NewRecorder()
h.ServeHTTP(w, buildWebhookRequest(string(body)))
if w.Code != http.StatusOK {
t.Errorf("status: got %d, want 200", w.Code)
}
select {
case <-dispatched:
t.Fatal("onMessage must not be called for plaintext event when encrypt key is configured")
case <-time.After(100 * time.Millisecond):
}
}
// --- Non-message event type ---
func TestWebhookHandler_NonMessageEvent_Ignored(t *testing.T) {
@@ -217,6 +301,18 @@ func TestWebhookHandler_InvalidJSON(t *testing.T) {
}
}
func TestWebhookHandler_RejectsOversizedBody(t *testing.T) {
h := NewWebhookHandler("", "", func(_ *MessageEvent) {
t.Fatal("onMessage must not be called for oversized body")
})
w := httptest.NewRecorder()
h.ServeHTTP(w, buildWebhookRequest(strings.Repeat("x", maxWebhookBodyBytes+1)))
if w.Code != http.StatusRequestEntityTooLarge {
t.Errorf("status: got %d, want 413", w.Code)
}
}
// --- Encrypted event ---
func TestWebhookHandler_EncryptedEvent_Decrypted(t *testing.T) {
@@ -281,6 +377,13 @@ func TestDecryptEvent_InvalidBase64(t *testing.T) {
}
}
func TestDecryptEvent_RejectsNonBlockMultipleCiphertext(t *testing.T) {
payload := base64.StdEncoding.EncodeToString([]byte("12345678901234567"))
if _, err := decryptEvent(payload, "key"); err == nil {
t.Fatal("expected error for non-block-multiple ciphertext")
}
}
func TestDecryptEvent_TooShort(t *testing.T) {
// Valid base64 but shorter than AES block size (16 bytes)
short := base64.StdEncoding.EncodeToString([]byte("short"))