fix(webhook): fail loud on malformed signing secret and add tests

Follow-up to #6013. The signing path silently fell back to using the raw
secret string as the HMAC key when a whsec_-prefixed secret had invalid
base64, producing signatures no receiver could verify with no server-side
signal.

- Extract resolveSigningKey helper that errors on invalid whsec_ base64
- Post returns that error (logged by the async dispatcher); ValidateSigningSecret
  rejects it at write time so a bad secret is never stored
- Fix stale comment referencing a nonexistent Authorization header
- Add Go tests: key derivation, secret validation, end-to-end signature
  round-trip, and the invariant that the secret never leaks into API responses

internal/webhook/validate.go

@@ -84,7 +84,9 @@ func ValidateURL(rawURL string) error {
 
 // ValidateSigningSecret checks that secret is either empty (allowed) or contains
 // only printable ASCII characters (0x20–0x7E), excluding all control characters
-// such as \r and \n, which would break the HTTP Authorization header.
+// such as \r and \n, which would corrupt the webhook signature headers. When the
+// secret uses the Standard Webhooks "whsec_<base64>" serialization, the base64
+// body must decode cleanly so signing cannot silently fall back to the wrong key.
 func ValidateSigningSecret(secret string) error {
 	if secret == "" {
 		return nil
@@ -94,5 +96,8 @@ func ValidateSigningSecret(secret string) error {
 			return status.Errorf(codes.InvalidArgument, "signing secret contains invalid character")
 		}
 	}
+	if _, err := resolveSigningKey(secret); err != nil {
+		return status.Errorf(codes.InvalidArgument, "%v", err)
+	}
 	return nil
 }

internal/webhook/webhook.go

@@ -88,6 +88,22 @@ type WebhookRequestPayload struct {
 	SigningSecret string `json:"-"`
 }
 
+// resolveSigningKey returns the raw HMAC key for a signing secret. Secrets using
+// the Standard Webhooks "whsec_<base64>" serialization are base64-decoded to their
+// raw bytes; any other secret is used as-is. It returns an error when a whsec_-prefixed
+// secret is not valid base64, so callers fail loudly instead of silently signing with
+// the wrong key (which would make every signature unverifiable by the receiver).
+func resolveSigningKey(secret string) ([]byte, error) {
+	if rest, ok := strings.CutPrefix(secret, "whsec_"); ok {
+		decoded, err := base64.StdEncoding.DecodeString(rest)
+		if err != nil {
+			return nil, errors.Wrap(err, "signing secret has whsec_ prefix but is not valid base64")
+		}
+		return decoded, nil
+	}
+	return []byte(secret), nil
+}
+
 // Post posts the message to webhook endpoint.
 func Post(requestPayload *WebhookRequestPayload) error {
 	body, err := json.Marshal(requestPayload)
@@ -103,16 +119,14 @@ func Post(requestPayload *WebhookRequestPayload) error {
 	req.Header.Set("Content-Type", "application/json")
 
 	if requestPayload.SigningSecret != "" {
+		key, err := resolveSigningKey(requestPayload.SigningSecret)
+		if err != nil {
+			return errors.Wrapf(err, "failed to derive signing key for webhook to %s", requestPayload.URL)
+		}
+
 		msgID := "msg_" + uuid.New().String()
 		timestamp := strconv.FormatInt(time.Now().Unix(), 10)
 
-		key := []byte(requestPayload.SigningSecret)
-		if strings.HasPrefix(requestPayload.SigningSecret, "whsec_") {
-			if decoded, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(requestPayload.SigningSecret, "whsec_")); err == nil {
-				key = decoded
-			}
-		}
-
 		mac := hmac.New(sha256.New, key)
 		mac.Write([]byte(msgID + "." + timestamp + "."))
 		mac.Write(body)

internal/webhook/webhook_test.go

@@ -1,6 +1,13 @@
 package webhook
 
 import (
+	"crypto/hmac"
+	"crypto/sha256"
+	"encoding/base64"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -11,3 +18,129 @@ func TestPostAsyncNilPayloadDoesNotPanic(t *testing.T) {
 		PostAsync(nil)
 	})
 }
+
+func TestResolveSigningKey(t *testing.T) {
+	rawKey := []byte("0123456789abcdef")
+	whsec := "whsec_" + base64.StdEncoding.EncodeToString(rawKey)
+
+	t.Run("plain secret used as-is", func(t *testing.T) {
+		key, err := resolveSigningKey("my-plain-secret")
+		require.NoError(t, err)
+		require.Equal(t, []byte("my-plain-secret"), key)
+	})
+
+	t.Run("whsec_ prefix is base64-decoded", func(t *testing.T) {
+		key, err := resolveSigningKey(whsec)
+		require.NoError(t, err)
+		require.Equal(t, rawKey, key)
+	})
+
+	t.Run("whsec_ with invalid base64 fails loudly", func(t *testing.T) {
+		_, err := resolveSigningKey("whsec_not!valid!base64!")
+		require.Error(t, err)
+	})
+}
+
+func TestValidateSigningSecret(t *testing.T) {
+	tests := []struct {
+		name    string
+		secret  string
+		wantErr bool
+	}{
+		{name: "empty is allowed", secret: "", wantErr: false},
+		{name: "printable ascii", secret: "abcDEF123!@#", wantErr: false},
+		{name: "valid whsec_", secret: "whsec_" + base64.StdEncoding.EncodeToString([]byte("key")), wantErr: false},
+		{name: "newline rejected", secret: "abc\ndef", wantErr: true},
+		{name: "carriage return rejected", secret: "abc\rdef", wantErr: true},
+		{name: "tab rejected", secret: "abc\tdef", wantErr: true},
+		{name: "non-ascii rejected", secret: "abc€def", wantErr: true},
+		{name: "whsec_ with invalid base64 rejected", secret: "whsec_not!base64", wantErr: true},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := ValidateSigningSecret(tc.secret)
+			if tc.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+// TestPostSignsRequest verifies the end-to-end Standard Webhooks signature so a
+// receiver following the documented verification recipe will accept our requests.
+func TestPostSignsRequest(t *testing.T) {
+	// httptest listens on 127.0.0.1, which the SSRF guard blocks by default.
+	prev := AllowPrivateIPs
+	AllowPrivateIPs = true
+	defer func() { AllowPrivateIPs = prev }()
+
+	rawKey := []byte("0123456789abcdef0123456789abcdef")
+	cases := []struct {
+		name   string
+		secret string
+		key    []byte
+	}{
+		{name: "plain secret", secret: "plain-secret-value", key: []byte("plain-secret-value")},
+		{name: "whsec_ secret", secret: "whsec_" + base64.StdEncoding.EncodeToString(rawKey), key: rawKey},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var gotID, gotTimestamp, gotSignature string
+			var gotBody []byte
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				gotID = r.Header.Get("webhook-id")
+				gotTimestamp = r.Header.Get("webhook-timestamp")
+				gotSignature = r.Header.Get("webhook-signature")
+				gotBody, _ = io.ReadAll(r.Body)
+				_, _ = w.Write([]byte(`{"code":0}`))
+			}))
+			defer server.Close()
+
+			err := Post(&WebhookRequestPayload{
+				URL:           server.URL,
+				ActivityType:  "memos.memo.created",
+				Creator:       "users/1",
+				SigningSecret: tc.secret,
+			})
+			require.NoError(t, err)
+
+			require.True(t, strings.HasPrefix(gotID, "msg_"), "webhook-id should be prefixed with msg_")
+			require.NotEmpty(t, gotTimestamp)
+			require.True(t, strings.HasPrefix(gotSignature, "v1,"), "signature should carry the v1 version tag")
+
+			// Recompute the signature the way a receiver would and confirm it matches.
+			mac := hmac.New(sha256.New, tc.key)
+			mac.Write([]byte(gotID + "." + gotTimestamp + "."))
+			mac.Write(gotBody)
+			want := base64.StdEncoding.EncodeToString(mac.Sum(nil))
+			require.Equal(t, "v1,"+want, gotSignature)
+		})
+	}
+}
+
+// TestPostWithoutSecretSetsNoSignatureHeaders ensures unsigned webhooks stay unsigned.
+func TestPostWithoutSecretSetsNoSignatureHeaders(t *testing.T) {
+	prev := AllowPrivateIPs
+	AllowPrivateIPs = true
+	defer func() { AllowPrivateIPs = prev }()
+
+	var hasSignatureHeaders bool
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		hasSignatureHeaders = r.Header.Get("webhook-id") != "" ||
+			r.Header.Get("webhook-timestamp") != "" ||
+			r.Header.Get("webhook-signature") != ""
+		_, _ = w.Write([]byte(`{"code":0}`))
+	}))
+	defer server.Close()
+
+	err := Post(&WebhookRequestPayload{
+		URL:          server.URL,
+		ActivityType: "memos.memo.created",
+		Creator:      "users/1",
+	})
+	require.NoError(t, err)
+	require.False(t, hasSignatureHeaders, "no signature headers should be set when no secret is configured")
+}

server/router/api/v1/user_webhook_test.go

@@ -0,0 +1,29 @@
+package v1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	storepb "github.com/usememos/memos/proto/gen/store"
+	"github.com/usememos/memos/store"
+)
+
+// TestConvertUserWebhookFromUserSettingOmitsSigningSecret guards the core security
+// invariant of the signing-secret feature: the secret is INPUT_ONLY and must never
+// be copied into an API response, even though it is persisted in the user setting.
+func TestConvertUserWebhookFromUserSettingOmitsSigningSecret(t *testing.T) {
+	user := &store.User{Username: "alice"}
+	stored := &storepb.WebhooksUserSetting_Webhook{
+		Id:            "webhook-id",
+		Title:         "My Webhook",
+		Url:           "https://example.com/postreceive",
+		SigningSecret: "whsec_super-secret-value",
+	}
+
+	apiWebhook := convertUserWebhookFromUserSetting(stored, user)
+
+	require.Equal(t, "My Webhook", apiWebhook.DisplayName)
+	require.Equal(t, "https://example.com/postreceive", apiWebhook.Url)
+	require.Empty(t, apiWebhook.SigningSecret, "signing secret must never be returned in API responses")
+}