fix(clear-rate-limit): match RATE_LIMIT_DOMAIN literally in the LIKE pattern (#1272)

clear-rate-limit built its DELETE pattern by concatenating
RATE_LIMIT_DOMAIN into "EMAIL#%@<domain>#ENDPOINT#forgot_password". The
query was parameterized, but % and _ inside the domain (and the _ in
"forgot_password") are LIKE wildcards, so RATE_LIMIT_DOMAIN=% cleared the
forgot_password rate limits of every domain, and a domain with an _
matched any character in that position.

The domain and endpoint are now escaped and the DELETE uses
ESCAPE '\', so only the email's local part is a wildcard. The query moved
into clearForgotPasswordLimits so a real-Postgres test can call it.

The cleanup-lambda half of the same review finding is not changed here:
its status guard already landed in #1173, and its retention rule is left
for the maintainer to decide (see the PR body).

Refs #357

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: IAmJSD

cmd/lambda/clear-rate-limit/main.go

@@ -5,13 +5,31 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"strings"
 
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/database"
 	"github.com/aws/aws-lambda-go/lambda"
 )
 
 const defaultDomain = "leanercloud.com"
 
+// forgotPasswordEndpoint is the rate-limit endpoint whose per-email rows this
+// function clears (see internal/api/rate_limiter.go).
+const forgotPasswordEndpoint = "forgot_password"
+
+// likeEscaper escapes the LIKE metacharacters (and the escape character
+// itself) so a value matches only itself inside a pattern that is used with
+// ESCAPE '\'.
+var likeEscaper = strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`)
+
+// forgotPasswordPattern is the LIKE pattern for every forgot_password row of
+// one email domain. Only the email's local part is a wildcard: the domain and
+// endpoint are escaped, so RATE_LIMIT_DOMAIN=% matches a literal "%" domain
+// instead of every domain, and an "_" matches only "_" (issue #357).
+func forgotPasswordPattern(domain string) string {
+	return "EMAIL#%@" + likeEscaper.Replace(domain) + "#ENDPOINT#" + likeEscaper.Replace(forgotPasswordEndpoint)
+}
+
 func getDomain() string {
 	if domain := os.Getenv("RATE_LIMIT_DOMAIN"); domain != "" {
 		return domain
@@ -35,11 +53,15 @@ func clearRateLimit(ctx context.Context) (Response, error) {
 	}
 	defer db.Close()
 
-	// Clear rate limits for forgot_password endpoint
-	domain := getDomain()
+	return clearForgotPasswordLimits(ctx, db, getDomain())
+}
+
+// clearForgotPasswordLimits deletes the forgot_password rate-limit rows of
+// every email at domain and reports how many rate-limit rows remain.
+func clearForgotPasswordLimits(ctx context.Context, db *database.Connection, domain string) (Response, error) {
 	tag, err := db.Exec(ctx,
-		"DELETE FROM rate_limits WHERE id LIKE $1",
-		"EMAIL#%@"+domain+"#ENDPOINT#forgot_password")
+		`DELETE FROM rate_limits WHERE id LIKE $1 ESCAPE '\'`,
+		forgotPasswordPattern(domain))
 	if err != nil {
 		return Response{}, fmt.Errorf("failed to delete rate limits: %w", err)
 	}

cmd/lambda/clear-rate-limit/main_integration_test.go

@@ -0,0 +1,119 @@
+//go:build integration
+
+package main
+
+// Real PostgreSQL tests for the clear-rate-limit DELETE (issue #357). The
+// migrated rate_limits table and the DELETE are real; nothing is deployed and
+// no Lambda is invoked.
+
+import (
+	"context"
+	"path/filepath"
+	"runtime"
+	"sort"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/database"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers"
+	"github.com/stretchr/testify/require"
+)
+
+// rateLimitRows covers the domain the tests clear, other domains, a literal
+// "%" domain, and rows of other endpoints and key kinds. The exampleQtest.com
+// and forgotXpassword rows match only while "_" is a wildcard.
+var rateLimitRows = []string{
+	"EMAIL#alice@example.com#ENDPOINT#forgot_password",
+	"EMAIL#bob@example.com#ENDPOINT#forgot_password",
+	"EMAIL#dave@other.org#ENDPOINT#forgot_password",
+	"EMAIL#erin@%#ENDPOINT#forgot_password",
+	"EMAIL#frank@example_test.com#ENDPOINT#forgot_password",
+	"EMAIL#grace@exampleQtest.com#ENDPOINT#forgot_password",
+	"EMAIL#alice@example.com#ENDPOINT#forgotXpassword",
+	"EMAIL#alice@example.com#ENDPOINT#login",
+	"IP#192.0.2.1#ENDPOINT#forgot_password_ip",
+}
+
+func rateLimitTestDB(ctx context.Context, t *testing.T) *database.Connection {
+	t.Helper()
+	c := testhelpers.RequirePostgresContainer(ctx, t)
+	t.Cleanup(func() { _ = c.Cleanup(context.Background()) })
+	_, file, _, _ := runtime.Caller(0)
+	path := filepath.Join(filepath.Dir(file), "..", "..", "..", "internal", "database", "postgres", "migrations")
+	require.NoError(t, migrations.RunMigrations(ctx, c.DB.Pool(), path, "", ""))
+	return c.DB
+}
+
+func seedRateLimits(ctx context.Context, t *testing.T, db *database.Connection) {
+	t.Helper()
+	_, err := db.Exec(ctx, `DELETE FROM rate_limits`)
+	require.NoError(t, err)
+	for _, id := range rateLimitRows {
+		_, err := db.Exec(ctx, `INSERT INTO rate_limits (id, count, reset_time) VALUES ($1, 1, NOW() + INTERVAL '1 hour')`, id)
+		require.NoError(t, err, "insert %s", id)
+	}
+}
+
+func remainingRateLimits(ctx context.Context, t *testing.T, db *database.Connection) []string {
+	t.Helper()
+	rows, err := db.Query(ctx, `SELECT id FROM rate_limits`)
+	require.NoError(t, err)
+	defer rows.Close()
+	var out []string
+	for rows.Next() {
+		var id string
+		require.NoError(t, rows.Scan(&id))
+		out = append(out, id)
+	}
+	require.NoError(t, rows.Err())
+	sort.Strings(out)
+	return out
+}
+
+// without returns rateLimitRows minus the given ids, sorted.
+func without(deleted ...string) []string {
+	gone := map[string]bool{}
+	for _, id := range deleted {
+		gone[id] = true
+	}
+	var out []string
+	for _, id := range rateLimitRows {
+		if !gone[id] {
+			out = append(out, id)
+		}
+	}
+	sort.Strings(out)
+	return out
+}
+
+// TestClearForgotPasswordLimitsDeletesOnlyTheNamedDomain is the issue's
+// failure scenario: RATE_LIMIT_DOMAIN=% used to clear the forgot_password rows
+// of every domain, and an "_" in the domain matched any one character.
+func TestClearForgotPasswordLimitsDeletesOnlyTheNamedDomain(t *testing.T) {
+	ctx := context.Background()
+	db := rateLimitTestDB(ctx, t)
+
+	cases := []struct {
+		domain  string
+		deleted []string
+	}{
+		{"example.com", []string{
+			"EMAIL#alice@example.com#ENDPOINT#forgot_password",
+			"EMAIL#bob@example.com#ENDPOINT#forgot_password",
+		}},
+		{"%", []string{"EMAIL#erin@%#ENDPOINT#forgot_password"}},
+		{"example_test.com", []string{"EMAIL#frank@example_test.com#ENDPOINT#forgot_password"}},
+		{"nobody.invalid", nil},
+	}
+	for _, tc := range cases {
+		t.Run(tc.domain, func(t *testing.T) {
+			seedRateLimits(ctx, t, db)
+			resp, err := clearForgotPasswordLimits(ctx, db, tc.domain)
+			require.NoError(t, err)
+			want := without(tc.deleted...)
+			require.Equal(t, want, remainingRateLimits(ctx, t, db))
+			require.Equal(t, len(tc.deleted), resp.DeletedCount)
+			require.Equal(t, len(want), resp.RemainingCount)
+		})
+	}
+}

cmd/lambda/clear-rate-limit/main_test.go

@@ -0,0 +1,23 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestForgotPasswordPatternEscapesLikeMetacharacters pins the pattern the
+// DELETE runs with ESCAPE '\': only the email's local part is a wildcard
+// (issue #357). The real-Postgres test in main_integration_test.go checks
+// which rows the pattern actually deletes.
+func TestForgotPasswordPatternEscapesLikeMetacharacters(t *testing.T) {
+	cases := map[string]string{
+		"example.com":      `EMAIL#%@example.com#ENDPOINT#forgot\_password`,
+		"%":                `EMAIL#%@\%#ENDPOINT#forgot\_password`,
+		"example_test.com": `EMAIL#%@example\_test.com#ENDPOINT#forgot\_password`,
+		`ex\ample.com`:     `EMAIL#%@ex\\ample.com#ENDPOINT#forgot\_password`,
+	}
+	for domain, want := range cases {
+		assert.Equal(t, want, forgotPasswordPattern(domain), "domain %q", domain)
+	}
+}