Tighten Google Chat webhook validation and settings response

Restrict the decoded key and token to a URL-safe ASCII alphabet so the
16-character minimum and the four-character masked tail measure the same
thing; a token of a few multi-byte characters could otherwise be shown in
full (MQ-165-01).

Validate the raw query as exactly key=…&token=…, refusing empty
segments, a second "?", reordered, extra or repeated parameters
(MQ-165-02).

Drop configured and updated_at from the settings response so it is the
four-field contract: space_label, url_masked, last_test_at,
last_test_status; url_masked is null exactly when nothing is stored
(MQ-165-03).

Task: 919e9422-3a41-4f1f-8967-77da17f5f9f3

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs/ARCHITECTURE.md

@@ -693,13 +693,16 @@ Runs are recorded in `schedule_runs` and executed through `jobs`.
     `{"url", "space_label"}`, both required. The URL must be exactly
     `https://chat.googleapis.com/v1/spaces/{space}/messages?key=…&token=…`:
     that scheme and host spelled so, no port or user info, the space id
-    letters, digits, `-` or `_`, and only the query parameters `key` and
+    letters, digits, `-` or `_`, and the raw query exactly
-    `token`, each once and at least 16 characters. The space label is 1–100
+    `key=…&token=…` (in that order, no empty segment, no second `?`, no
+    other parameter), each decoding to at least 16 characters of letters,
+    digits, `-`, `_`, `.`, `~` or `=`. The space label is 1–100
     characters. A refused body is 400 `invalid_request` (not a JSON object
     of the two fields) or `invalid_webhook`. Saving replaces any stored
     webhook and clears its test outcome.
-  - `GET /api/v1/settings/chat-webhook` answers `{configured, space_label,
+  - `GET /api/v1/settings/chat-webhook` answers `{space_label, url_masked,
-    url_masked, last_test_at, last_test_status, updated_at}`; `url_masked`
+    last_test_at, last_test_status}`, so `url_masked` is `null` exactly when
+    no webhook is stored; `url_masked`
     shows the space id and only the last four characters of the decoded
     token (`…/spaces/AAQA…/messages?key=…&token=…ZxQ=`). Absent values are
     `null`.

internal/chat/chat.go

@@ -59,9 +59,8 @@ var (
 )
 
 // Settings is the stored webhook as the API shows it: never the URL, only
-// its masked form.
+// its masked form. URLMasked is nil when no webhook is stored.
 type Settings struct {
-	Configured bool    `json:"configured"`
 	SpaceLabel *string `json:"space_label"`
 	// URLMasked is the webhook URL without its key and with only the last
 	// few characters of its token.
@@ -70,7 +69,6 @@ type Settings struct {
 	// test message is sent to this webhook.
 	LastTestAt     *string `json:"last_test_at"`
 	LastTestStatus *string `json:"last_test_status"`
-	UpdatedAt      *string `json:"updated_at"`
 }
 
 // TestResult is the outcome of a test message.
@@ -143,8 +141,8 @@ func NewService(st *store.Store, box *secretbox.Box, publicURL *url.URL, transpo
 	}, nil
 }
 
-// Get returns the stored webhook's settings; Configured is false and every
+// Get returns the stored webhook's settings; every field is nil before one
-// other field nil before one is saved.
+// is saved.
 func (s *Service) Get(ctx context.Context) (Settings, error) {
 	row, err := db.New(s.st.Reader).GetChatSettings(ctx)
 	if errors.Is(err, sql.ErrNoRows) {
@@ -440,14 +438,12 @@ func (s *Service) timestamp() string {
 }
 
 func settingsFrom(row db.ChatSetting, hook Webhook) Settings {
-	masked, updated := hook.Masked(), row.UpdatedAt
+	masked := hook.Masked()
 	return Settings{
-		Configured:     true,
 		SpaceLabel:     nullString(row.SpaceLabel),
 		URLMasked:      &masked,
 		LastTestAt:     nullString(row.LastTestAt),
 		LastTestStatus: nullString(row.LastTestStatus),
-		UpdatedAt:      &updated,
 	}
 }
 

internal/chat/chat_test.go

@@ -172,7 +172,7 @@ func TestPutGetStoresSealedWebhook(t *testing.T) {
 	svc, st := newTestService(t, newFakeChat(t))
 
 	got, err := svc.Get(ctx)
-	if err != nil || got.Configured || got.SpaceLabel != nil || got.URLMasked != nil || got.LastTestAt != nil {
+	if err != nil || got.SpaceLabel != nil || got.URLMasked != nil || got.LastTestAt != nil {
 		t.Fatalf("Get unconfigured = %+v, %v", got, err)
 	}
 
@@ -180,7 +180,7 @@ func TestPutGetStoresSealedWebhook(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !put.Configured || *put.SpaceLabel != "Family dinner" || *put.URLMasked != "https://chat.googleapis.com/v1/spaces/"+testSpace+"/messages?key=…&token=…ZxQ=" || put.LastTestAt != nil || put.LastTestStatus != nil {
+	if *put.SpaceLabel != "Family dinner" || *put.URLMasked != "https://chat.googleapis.com/v1/spaces/"+testSpace+"/messages?key=…&token=…ZxQ=" || put.LastTestAt != nil || put.LastTestStatus != nil {
 		t.Errorf("Put = %+v", put)
 	}
 	got, err = svc.Get(ctx)

internal/chat/webhook.go

@@ -28,6 +28,10 @@ const (
 // space id.
 var webhookPath = regexp.MustCompile(`^/v1/spaces/([A-Za-z0-9_-]{1,128})/messages$`)
 
+// credentialChars is the alphabet of a decoded key or token: URL-safe
+// ASCII, so its length in bytes is its length in characters.
+var credentialChars = regexp.MustCompile(`^[A-Za-z0-9_.~=-]*$`)
+
 // Webhook is a validated Google Chat incoming webhook URL. Its String is
 // the full URL, a secret: it is only ever sealed for storage or sent to
 // Google Chat, never logged, shown or put in an error.
@@ -39,8 +43,9 @@ type Webhook struct {
 
 // ParseWebhook validates raw as
 // https://chat.googleapis.com/v1/spaces/{space}/messages?key=…&token=…,
-// with exactly the query parameters key and token, each once and at least
+// whose raw query is exactly key=…&token=…, in that order, each decoding to
-// MinCredentialLength characters. Its error never quotes raw.
+// at least MinCredentialLength characters of credentialChars. Its error
+// never quotes raw.
 func ParseWebhook(raw string) (Webhook, error) {
 	switch {
 	case raw == "":
@@ -65,28 +70,34 @@ func ParseWebhook(raw string) (Webhook, error) {
 	if m == nil {
 		return Webhook{}, errors.New("path must be /v1/spaces/{space}/messages")
 	}
-	q, err := url.ParseQuery(u.RawQuery)
+	// The raw query is exactly key=…&token=…: no empty segment, no second
-	if err != nil {
+	// "?", no other or repeated parameter.
-		return Webhook{}, errors.New("query is not valid")
+	if strings.Contains(u.RawQuery, "?") {
+		return Webhook{}, errors.New("query must not contain ?")
 	}
-	for name, values := range q {
+	segments := strings.Split(u.RawQuery, "&")
-		if name != "key" && name != "token" {
+	if len(segments) != 2 {
-			return Webhook{}, errors.New("query must hold only key and token")
+		return Webhook{}, errors.New("query must be exactly key=…&token=…")
-		}
-		if len(values) != 1 {
-			return Webhook{}, fmt.Errorf("query must hold %s once", name)
-		}
 	}
-	for _, name := range []string{"key", "token"} {
+	creds := make([]string, 2)
-		v := q.Get(name)
+	for i, name := range []string{"key", "token"} {
-		if v == "" {
+		escaped, ok := strings.CutPrefix(segments[i], name+"=")
-			return Webhook{}, fmt.Errorf("query must hold a %s", name)
+		if !ok {
+			return Webhook{}, errors.New("query must be exactly key=…&token=…")
+		}
+		v, err := url.QueryUnescape(escaped)
+		if err != nil {
+			return Webhook{}, errors.New("query is not valid")
+		}
+		if !credentialChars.MatchString(v) {
+			return Webhook{}, fmt.Errorf("%s must be letters, digits, -, _, ., ~ or =", name)
 		}
 		if len(v) < MinCredentialLength {
 			return Webhook{}, fmt.Errorf("%s must be at least %d characters", name, MinCredentialLength)
 		}
+		creds[i] = v
 	}
-	return Webhook{raw: raw, space: m[1], token: q.Get("token")}, nil
+	return Webhook{raw: raw, space: m[1], token: creds[1]}, nil
 }
 
 // String returns the full webhook URL, a secret.
@@ -98,7 +109,7 @@ func (w Webhook) String() string {
 // characters of its token, such as
 // "https://chat.googleapis.com/v1/spaces/AAQA1234/messages?key=…&token=…x3Dk".
 func (w Webhook) Masked() string {
-	// MinCredentialLength bytes are at least maskedTail runes.
+	// The token is ASCII of at least MinCredentialLength bytes, so the
-	token := []rune(w.token)
+	// tail is maskedTail of its many characters.
-	return webhookPrefix + w.space + "/messages?key=…&token=…" + string(token[len(token)-maskedTail:])
+	return webhookPrefix + w.space + "/messages?key=…&token=…" + w.token[len(w.token)-maskedTail:]
 }

internal/chat/webhook_test.go

@@ -16,7 +16,6 @@ func TestParseWebhookAccepts(t *testing.T) {
 	t.Parallel()
 	for _, raw := range []string{
 		testHook,
-		"https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?token=" + testToken + "&key=" + testKey,
 		"https://chat.googleapis.com/v1/spaces/AAAA_x-y/messages?key=" + testKey + "&token=" + testToken,
 	} {
 		hook, err := ParseWebhook(raw)
@@ -55,6 +54,14 @@ func TestParseWebhookRejects(t *testing.T) {
 		"no token":            "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?key=" + testKey,
 		"empty token":         "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?key=" + testKey + "&token=",
 		"short key":           "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?key=abc&token=" + testToken,
+		"token first":         "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?token=" + testToken + "&key=" + testKey,
+		"empty segment":       "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?key=" + testKey + "&&token=" + testToken,
+		"leading ampersand":   "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?&" + q[1:],
+		"trailing ampersand":  "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages" + q + "&",
+		"second question":     "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages" + q + "?x",
+		"multi-byte token":    "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?key=" + testKey + "&token=" + strings.Repeat("%F0%9F%98%80", 4),
+		"multi-byte key":      "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?key=" + strings.Repeat("%F0%9F%98%80", 4) + "&token=" + testToken,
+		"plus in token":       "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages?key=" + testKey + "&token=" + testToken + "+",
 		"repeated key":        "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages" + q + "&key=" + testKey,
 		"extra parameter":     "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages" + q + "&threadKey=x",
 		"bad escape in query": "https://chat.googleapis.com/v1/spaces/" + testSpace + "/messages" + q + "%zz",

internal/server/chat_test.go

@@ -131,7 +131,7 @@ func TestChatWebhookAPI(t *testing.T) {
 	}
 
 	rec, body := call(http.MethodGet, chatWebhookPath, "")
-	if rec.Code != http.StatusOK || body["configured"] != false || body["url_masked"] != nil || body["space_label"] != nil {
+	if rec.Code != http.StatusOK || body["url_masked"] != nil || body["space_label"] != nil || body["last_test_at"] != nil || body["last_test_status"] != nil || len(body) != 4 {
 		t.Errorf("GET unconfigured = %d %v", rec.Code, body)
 	}
 	rec, body = call(http.MethodPost, chatWebhookTestPath, "")
@@ -160,8 +160,8 @@ func TestChatWebhookAPI(t *testing.T) {
 
 	rec, body = call(http.MethodPut, chatWebhookPath, `{"url":"`+testChatHook+`","space_label":"Family dinner"}`)
 	wantMasked := "https://chat.googleapis.com/v1/spaces/AAQA1b2C3d4/messages?key=…&token=…ZxQ="
-	if rec.Code != http.StatusOK || body["configured"] != true || body["space_label"] != "Family dinner" || body["url_masked"] != wantMasked ||
+	if rec.Code != http.StatusOK || body["space_label"] != "Family dinner" || body["url_masked"] != wantMasked ||
-		body["last_test_at"] != nil || body["last_test_status"] != nil {
+		body["last_test_at"] != nil || body["last_test_status"] != nil || len(body) != 4 {
 		t.Errorf("PUT = %d %v", rec.Code, body)
 	}
 	if _, ok := body["url"]; ok {