From a65442ce57d64a160c6483b90257324a7fa84feb Mon Sep 17 00:00:00 2001 From: Paul Schifferer Date: Tue, 6 Oct 2026 21:49:44 -0700 Subject: [PATCH 1/6] feat(feedback): add POST /feedback anonymous GitHub issue proxy Implements group 2 of the add-anonymous-feedback-reporting OpenSpec change: a POST /feedback handler accepting type (bug|feature), title, body, and optional reporter_email/source, that files a labeled GitHub Issue on sweetrpg/platform via a new feedback package wrapping the GitHub Issues API (go-github). GitHub failures/timeouts return 502 without retrying. Adds a feedback-specific Redis-backed rate-limit tier (api-core.go/ratelimit, bumped to v0.3.1) stricter than and layered on top of the service-wide limiter, scoped to only this route. Adds title/body length caps and a honeypot + minimum-fill-time bot-detection check that silently accepts (200, no issue created) instead of rejecting, so a bot gets no signal. Adds swaggo annotations and regenerates docs/swagger.{json,yaml,go}. --- cmd/admin-api/main.go | 46 ++++- constants/constants.go | 34 +++ docs/docs.go | 103 +++++++++ docs/swagger.json | 103 +++++++++ docs/swagger.yaml | 76 +++++++ feedback/client.go | 62 ++++++ feedback/client_test.go | 69 +++++++ feedback/ratelimit.go | 62 ++++++ go.mod | 7 +- go.sum | 16 +- server/feedback.go | 130 ++++++++++++ server/feedback_test.go | 287 ++++++++++++++++++++++++++ server/handlers.go | 4 +- server/write_auth_integration_test.go | 9 +- 14 files changed, 1002 insertions(+), 6 deletions(-) create mode 100644 feedback/client.go create mode 100644 feedback/client_test.go create mode 100644 feedback/ratelimit.go create mode 100644 server/feedback.go create mode 100644 server/feedback_test.go diff --git a/cmd/admin-api/main.go b/cmd/admin-api/main.go index 2a0c471..9a13f37 100644 --- a/cmd/admin-api/main.go +++ b/cmd/admin-api/main.go @@ -14,6 +14,7 @@ import ( "github.com/getsentry/sentry-go" "github.com/gin-contrib/cors" "github.com/gin-gonic/gin" + "github.com/gomodule/redigo/redis" "github.com/grafana/pyroscope-go" "github.com/joho/godotenv" "github.com/penglongli/gin-metrics/ginmetrics" @@ -24,6 +25,7 @@ import ( "github.com/sweetrpg/admin-api/authz" "github.com/sweetrpg/admin-api/constants" "github.com/sweetrpg/admin-api/docs" + "github.com/sweetrpg/admin-api/feedback" "github.com/sweetrpg/admin-api/models" "github.com/sweetrpg/admin-api/server" apiconstants "github.com/sweetrpg/api-core.go/constants" @@ -37,6 +39,10 @@ import ( "golang.org/x/time/rate" ) +// redisConnectTimeout bounds the feedback rate limiter's Redis connection attempts so a +// stalled connection fails fast instead of hanging past the caller's own timeout. +const redisConnectTimeout = 5 * time.Second + // @title Admin API service // @version 1.0 // @description Swagger APIs @@ -101,8 +107,14 @@ func main() { // Add rate limiter r.Use(RateLimiter()) + redisPool := setupRedisPool() + if redisPool != nil { + defer func() { _ = redisPool.Close() }() + } + authzClient := authz.NewClient(util.GetEnv(constants.AUTH_API_URL, "")) - server.SetupHandlers(r, authzClient) + issueCreator := feedback.NewGitHubIssueClient(util.GetEnv(constants.GITHUB_FEEDBACK_TOKEN, "")) + server.SetupHandlers(r, authzClient, issueCreator, feedback.RateLimitMiddleware(redisPool)) _ = r.Run(util.GetEnv(apiconstants.BIND_ADDRESS, ":8000")) } @@ -251,6 +263,38 @@ func setupMetrics(r *gin.Engine) { m.Use(r) } +// setupRedisPool builds a shared redigo connection pool for the feedback endpoint's dedicated +// rate limiter, when REDIS_HOST is configured. Returns nil when no Redis is configured, in +// which case feedback.RateLimitMiddleware fails closed (503) rather than running unlimited. +func setupRedisPool() *redis.Pool { + redisHost, found := os.LookupEnv(apiconstants.REDIS_HOST) + if !found { + return nil + } + + redisPort := util.GetEnv(apiconstants.REDIS_PORT, "6379") + redisPass := os.Getenv(apiconstants.REDIS_PASS) + addr := fmt.Sprintf("%s:%s", redisHost, redisPort) + + return &redis.Pool{ + MaxIdle: 5, + IdleTimeout: 240 * time.Second, + Dial: func() (redis.Conn, error) { + c, err := redis.Dial("tcp", addr, redis.DialConnectTimeout(redisConnectTimeout)) + if err != nil { + return nil, err + } + if redisPass != "" { + if _, err := c.Do("AUTH", redisPass); err != nil { + _ = c.Close() + return nil, err + } + } + return c, nil + }, + } +} + func RateLimiter() gin.HandlerFunc { limiter := rate.NewLimiter(1, util.GetEnvInt(apiconstants.RATE_LIMIT, 10)) diff --git a/constants/constants.go b/constants/constants.go index 2d46f56..768f3e4 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -1,5 +1,7 @@ package constants +import "time" + // Environment variable names const ( HEALTH_TOKEN = "HEALTH_TOKEN" @@ -10,6 +12,16 @@ const ( // AUTH_API_URL is auth-api's base URL, used by server/middleware.WriteAuth to verify // forwarded user bearer tokens via /authz/check. AUTH_API_URL = "AUTH_API_URL" + + // GITHUB_FEEDBACK_TOKEN authenticates the feedback package's GitHub Issues API calls. + // Scoped to Issues:write on sweetrpg/platform only - see design.md. + GITHUB_FEEDBACK_TOKEN = "GITHUB_FEEDBACK_TOKEN" + + // RATE_LIMIT_FEEDBACK and RATE_LIMIT_FEEDBACK_WINDOW_SECONDS tune the feedback-specific + // rate-limit tier, stricter than api-core.go/ratelimit's standard tier. See + // feedback.RateLimitMiddleware. + RATE_LIMIT_FEEDBACK = "RATE_LIMIT_FEEDBACK" + RATE_LIMIT_FEEDBACK_WINDOW_SECONDS = "RATE_LIMIT_FEEDBACK_WINDOW_SECONDS" ) // Value constants @@ -36,4 +48,26 @@ const ( // AdminActionAuditLogCollection is the MongoDB collection name for write-route // audit records. AdminActionAuditLogCollection = "admin_action_audit_logs" + + // RateLimitFeedbackDefault and RateLimitFeedbackWindowDefault are the feedback tier's + // fallback budget when RATE_LIMIT_FEEDBACK[_WINDOW_SECONDS] aren't set: 5 submissions + // per 5 minutes per client, well under the platform-default standard tier (see + // design.md's "dedicated feedback-specific rate-limit tier" decision). + RateLimitFeedbackDefault = 5 + RateLimitFeedbackWindowDefault = 300 + + // FeedbackTitleMaxLength and FeedbackBodyMaxLength cap submitted text, rejected with a + // 400 when exceeded (see the anonymous-feedback-submission spec's "Oversized submission + // rejected" scenario). + FeedbackTitleMaxLength = 200 + FeedbackBodyMaxLength = 10000 + + // FeedbackMinFillTime is the minimum time between the form rendering (client-reported + // rendered_at) and submission for a human to plausibly have filled it out. Submissions + // faster than this are silently discarded - see design.md's honeypot/fill-time decision. + FeedbackMinFillTime = 3 * time.Second + + // FeedbackSourceLabel is applied to every issue created from a feedback submission, so a + // spam wave can be bulk-filtered/closed without affecting other issues. + FeedbackSourceLabel = "source:in-app-feedback" ) diff --git a/docs/docs.go b/docs/docs.go index 340e8ac..45e2af9 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -524,6 +524,64 @@ const docTemplate = `{ } } }, + "/feedback": { + "post": { + "description": "Accept an anonymous bug report or feature request and file it as a GitHub Issue on sweetrpg/platform, labeled source:in-app-feedback plus bug or feature. No GitHub credentials are ever requested from or exposed to the caller. Subject to a feedback-specific rate limit stricter than this service's default.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "feedback" + ], + "summary": "Submit feedback", + "parameters": [ + { + "description": "Feedback submission", + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/server.createFeedbackRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/server.feedbackAcceptedResponse" + } + }, + "400": { + "description": "missing/invalid type, title, or body, or title/body exceeds the length cap", + "schema": { + "$ref": "#/definitions/vo.ErrorVO" + } + }, + "429": { + "description": "feedback-specific rate limit exceeded", + "schema": { + "$ref": "#/definitions/vo.ErrorVO" + } + }, + "502": { + "description": "GitHub API call failed or timed out", + "schema": { + "$ref": "#/definitions/vo.ErrorVO" + } + }, + "503": { + "description": "rate-limit backend unavailable", + "schema": { + "$ref": "#/definitions/vo.ErrorVO" + } + } + } + } + }, "/maintenance-modes": { "get": { "description": "Returns every maintenance-mode record (platform and every known service scope) regardless of enabled state, for admin-web's management view.", @@ -1031,6 +1089,51 @@ const docTemplate = `{ } } }, + "server.createFeedbackRequest": { + "type": "object", + "required": [ + "body", + "title", + "type" + ], + "properties": { + "body": { + "type": "string", + "example": "Searching for 9780786965601 returns zero results, even though the volume exists in the catalog." + }, + "rendered_at": { + "type": "string" + }, + "reporter_email": { + "type": "string", + "example": "reporter@example.com" + }, + "source": { + "type": "string", + "example": "catalog-web:/catalog/search" + }, + "title": { + "type": "string", + "example": "Catalog search returns no results for exact ISBN match" + }, + "type": { + "type": "string", + "example": "bug" + }, + "website": { + "type": "string" + } + } + }, + "server.feedbackAcceptedResponse": { + "type": "object", + "properties": { + "status": { + "type": "string", + "example": "accepted" + } + } + }, "server.maintenanceModeRequest": { "type": "object", "required": [ diff --git a/docs/swagger.json b/docs/swagger.json index 1a36e71..dbed88a 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -516,6 +516,64 @@ } } }, + "/feedback": { + "post": { + "description": "Accept an anonymous bug report or feature request and file it as a GitHub Issue on sweetrpg/platform, labeled source:in-app-feedback plus bug or feature. No GitHub credentials are ever requested from or exposed to the caller. Subject to a feedback-specific rate limit stricter than this service's default.", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "feedback" + ], + "summary": "Submit feedback", + "parameters": [ + { + "description": "Feedback submission", + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/server.createFeedbackRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/server.feedbackAcceptedResponse" + } + }, + "400": { + "description": "missing/invalid type, title, or body, or title/body exceeds the length cap", + "schema": { + "$ref": "#/definitions/vo.ErrorVO" + } + }, + "429": { + "description": "feedback-specific rate limit exceeded", + "schema": { + "$ref": "#/definitions/vo.ErrorVO" + } + }, + "502": { + "description": "GitHub API call failed or timed out", + "schema": { + "$ref": "#/definitions/vo.ErrorVO" + } + }, + "503": { + "description": "rate-limit backend unavailable", + "schema": { + "$ref": "#/definitions/vo.ErrorVO" + } + } + } + } + }, "/maintenance-modes": { "get": { "description": "Returns every maintenance-mode record (platform and every known service scope) regardless of enabled state, for admin-web's management view.", @@ -1023,6 +1081,51 @@ } } }, + "server.createFeedbackRequest": { + "type": "object", + "required": [ + "body", + "title", + "type" + ], + "properties": { + "body": { + "type": "string", + "example": "Searching for 9780786965601 returns zero results, even though the volume exists in the catalog." + }, + "rendered_at": { + "type": "string" + }, + "reporter_email": { + "type": "string", + "example": "reporter@example.com" + }, + "source": { + "type": "string", + "example": "catalog-web:/catalog/search" + }, + "title": { + "type": "string", + "example": "Catalog search returns no results for exact ISBN match" + }, + "type": { + "type": "string", + "example": "bug" + }, + "website": { + "type": "string" + } + } + }, + "server.feedbackAcceptedResponse": { + "type": "object", + "properties": { + "status": { + "type": "string", + "example": "accepted" + } + } + }, "server.maintenanceModeRequest": { "type": "object", "required": [ diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 6116111..7038d9d 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -138,6 +138,39 @@ definitions: - scope_type - severity type: object + server.createFeedbackRequest: + properties: + body: + example: Searching for 9780786965601 returns zero results, even though the + volume exists in the catalog. + type: string + rendered_at: + type: string + reporter_email: + example: reporter@example.com + type: string + source: + example: catalog-web:/catalog/search + type: string + title: + example: Catalog search returns no results for exact ISBN match + type: string + type: + example: bug + type: string + website: + type: string + required: + - body + - title + - type + type: object + server.feedbackAcceptedResponse: + properties: + status: + example: accepted + type: string + type: object server.maintenanceModeRequest: properties: description: @@ -577,6 +610,49 @@ paths: summary: Update a banner message tags: - banners + /feedback: + post: + consumes: + - application/json + description: Accept an anonymous bug report or feature request and file it as + a GitHub Issue on sweetrpg/platform, labeled source:in-app-feedback plus bug + or feature. No GitHub credentials are ever requested from or exposed to the + caller. Subject to a feedback-specific rate limit stricter than this service's + default. + parameters: + - description: Feedback submission + in: body + name: body + required: true + schema: + $ref: '#/definitions/server.createFeedbackRequest' + produces: + - application/json + responses: + "200": + description: OK + schema: + $ref: '#/definitions/server.feedbackAcceptedResponse' + "400": + description: missing/invalid type, title, or body, or title/body exceeds + the length cap + schema: + $ref: '#/definitions/vo.ErrorVO' + "429": + description: feedback-specific rate limit exceeded + schema: + $ref: '#/definitions/vo.ErrorVO' + "502": + description: GitHub API call failed or timed out + schema: + $ref: '#/definitions/vo.ErrorVO' + "503": + description: rate-limit backend unavailable + schema: + $ref: '#/definitions/vo.ErrorVO' + summary: Submit feedback + tags: + - feedback /maintenance-modes: get: description: Returns every maintenance-mode record (platform and every known diff --git a/feedback/client.go b/feedback/client.go new file mode 100644 index 0000000..b290eb7 --- /dev/null +++ b/feedback/client.go @@ -0,0 +1,62 @@ +// Package feedback implements the anonymous-feedback-to-GitHub-Issue proxy: a single +// IssueCreator interface backed by the GitHub Issues API, kept separate from the HTTP handler +// in server/feedback.go so the handler can be unit-tested against a fake creator. +package feedback + +import ( + "context" + "fmt" + "net/http" + "time" + + "github.com/google/go-github/v76/github" +) + +// Owner and Repo identify the sweetrpg/platform meta-repo that every feedback submission +// becomes an issue on - see design.md's "no per-service repo routing" decision. +const ( + Owner = "sweetrpg" + Repo = "platform" + + // createIssueTimeout bounds the GitHub API call so a stalled connection fails fast and + // the caller gets a prompt 502 rather than hanging past their own timeout. + createIssueTimeout = 10 * time.Second +) + +// IssueCreator creates a GitHub Issue from a feedback submission. Implemented by +// GitHubIssueClient; tests substitute a fake. +type IssueCreator interface { + CreateIssue(ctx context.Context, title, body string, labels []string) error +} + +// GitHubIssueClient creates issues on sweetrpg/platform via the GitHub REST API using a +// service-owned token scoped to Issues:write on that one repo (see design.md). +type GitHubIssueClient struct { + client *github.Client +} + +// NewGitHubIssueClient builds a client authenticated with the given token. +func NewGitHubIssueClient(token string) *GitHubIssueClient { + return &GitHubIssueClient{client: github.NewClient(nil).WithAuthToken(token)} +} + +// CreateIssue creates a labeled issue on sweetrpg/platform. A non-nil error means the caller +// should treat the submission as failed (502/503) - this never retries internally, per +// design.md's "no partial state to reconcile" decision. +func (c *GitHubIssueClient) CreateIssue(ctx context.Context, title, body string, labels []string) error { + ctx, cancel := context.WithTimeout(ctx, createIssueTimeout) + defer cancel() + + _, resp, err := c.client.Issues.Create(ctx, Owner, Repo, &github.IssueRequest{ + Title: &title, + Body: &body, + Labels: &labels, + }) + if err != nil { + return fmt.Errorf("feedback: create github issue: %w", err) + } + if resp.StatusCode != http.StatusCreated { + return fmt.Errorf("feedback: create github issue: unexpected status %d", resp.StatusCode) + } + return nil +} diff --git a/feedback/client_test.go b/feedback/client_test.go new file mode 100644 index 0000000..2a59625 --- /dev/null +++ b/feedback/client_test.go @@ -0,0 +1,69 @@ +package feedback + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/google/go-github/v76/github" +) + +func newTestClient(t *testing.T, handler http.HandlerFunc) *GitHubIssueClient { + t.Helper() + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + + gh := github.NewClient(nil).WithAuthToken("test-token") + base, err := url.Parse(server.URL + "/") + if err != nil { + t.Fatalf("parse test server URL: %v", err) + } + gh.BaseURL = base + + return &GitHubIssueClient{client: gh} +} + +func TestCreateIssue_Success(t *testing.T) { + var gotPath string + var gotBody github.IssueRequest + + client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil { + t.Fatalf("decode request body: %v", err) + } + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(github.Issue{Number: github.Ptr(1)}) + }) + + err := client.CreateIssue(context.Background(), "Bug: thing broke", "Steps to reproduce...", []string{"source:in-app-feedback", "bug"}) + if err != nil { + t.Fatalf("CreateIssue returned error: %v", err) + } + + wantPath := "/repos/" + Owner + "/" + Repo + "/issues" + if gotPath != wantPath { + t.Errorf("request path = %q, want %q", gotPath, wantPath) + } + if gotBody.Title == nil || *gotBody.Title != "Bug: thing broke" { + t.Errorf("request title = %v, want %q", gotBody.Title, "Bug: thing broke") + } + if gotBody.Labels == nil || len(*gotBody.Labels) != 2 || (*gotBody.Labels)[0] != "source:in-app-feedback" || (*gotBody.Labels)[1] != "bug" { + t.Errorf("request labels = %v, want [source:in-app-feedback bug]", gotBody.Labels) + } +} + +func TestCreateIssue_GitHubError(t *testing.T) { + client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"message":"internal error"}`)) + }) + + err := client.CreateIssue(context.Background(), "Bug", "Body", []string{"source:in-app-feedback", "bug"}) + if err == nil { + t.Fatal("CreateIssue returned nil error, want non-nil on GitHub API failure") + } +} diff --git a/feedback/ratelimit.go b/feedback/ratelimit.go new file mode 100644 index 0000000..c4c1864 --- /dev/null +++ b/feedback/ratelimit.go @@ -0,0 +1,62 @@ +package feedback + +import ( + "net/http" + + "github.com/gin-gonic/gin" + "github.com/gomodule/redigo/redis" + apicoreconstants "github.com/sweetrpg/api-core.go/constants" + "github.com/sweetrpg/api-core.go/ratelimit" + "github.com/sweetrpg/api-core.go/util" + apiv "github.com/sweetrpg/api-core.go/vo" + "github.com/sweetrpg/admin-api/constants" + "github.com/sweetrpg/common.go/logging" +) + +// tierFeedback is this route's own rate-limit tier name, kept separate from +// api-core.go/ratelimit's cheap/standard tiers so it doesn't change the budget for any other +// admin-api route (see design.md's "dedicated feedback-specific rate-limit tier" decision). +const tierFeedback = "feedback" + +// RateLimitMiddleware enforces a per-client budget on just the route it's mounted on, stricter +// than and layered on top of admin-api's existing service-wide limiter. Fails closed (503) if +// pool is nil or Redis is unreachable, matching api-core.go/ratelimit.Middleware's behavior. +func RateLimitMiddleware(pool *redis.Pool) gin.HandlerFunc { + limiter := ratelimit.New(pool, map[string]ratelimit.Tier{ + tierFeedback: { + Limit: util.GetEnvInt(constants.RATE_LIMIT_FEEDBACK, constants.RateLimitFeedbackDefault), + Window: util.GetEnvInt(constants.RATE_LIMIT_FEEDBACK_WINDOW_SECONDS, constants.RateLimitFeedbackWindowDefault), + }, + }) + + return func(c *gin.Context) { + if pool == nil { + logging.Logger.Error("Feedback rate-limit store not configured; rejecting request (fail closed)") + c.AbortWithStatusJSON(http.StatusServiceUnavailable, apiv.ErrorVO{ + Error: apicoreconstants.ErrorRateLimitUnavailable, + Message: "Rate limiting is temporarily unavailable", + }) + return + } + + clientKey := ratelimit.ClientKey(c) + allowed, err := limiter.Allow(c.Request.Context(), clientKey, tierFeedback) + if err != nil { + logging.Logger.Error("Feedback rate-limit backend unreachable; rejecting request (fail closed)", "error", err.Error()) + c.AbortWithStatusJSON(http.StatusServiceUnavailable, apiv.ErrorVO{ + Error: apicoreconstants.ErrorRateLimitUnavailable, + Message: "Rate limiting is temporarily unavailable", + }) + return + } + if !allowed { + logging.Logger.Warn("Feedback rate limit exceeded", "client", clientKey) + c.AbortWithStatusJSON(http.StatusTooManyRequests, apiv.ErrorVO{ + Error: apicoreconstants.ErrorRateLimited, + Message: "Limit exceeded", + }) + return + } + c.Next() + } +} diff --git a/go.mod b/go.mod index 67f9ccd..9306331 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,12 @@ module github.com/sweetrpg/admin-api go 1.26.5 require ( + github.com/alicebob/miniredis/v2 v2.39.0 github.com/getsentry/sentry-go v0.49.0 github.com/gin-contrib/cors v1.7.7 github.com/gin-gonic/gin v1.12.0 + github.com/gomodule/redigo v1.9.3 + github.com/google/go-github/v76 v76.0.0 github.com/grafana/pyroscope-go v1.4.2 github.com/joho/godotenv v1.5.1 github.com/penglongli/gin-metrics v0.1.13 @@ -14,7 +17,7 @@ require ( github.com/swaggo/files v1.0.1 github.com/swaggo/gin-swagger v1.6.1 github.com/swaggo/swag v1.16.6 - github.com/sweetrpg/api-core.go v0.1.1 + github.com/sweetrpg/api-core.go v0.3.1 github.com/sweetrpg/common.go v0.0.16 github.com/sweetrpg/mongodb.go v0.0.193 go.mongodb.org/mongo-driver v1.17.9 @@ -55,6 +58,7 @@ require ( github.com/goccy/go-json v0.10.6 // indirect github.com/goccy/go-yaml v1.19.2 // indirect github.com/golang/snappy v1.0.0 // indirect + github.com/google/go-querystring v1.1.0 // indirect github.com/google/uuid v1.6.0 // indirect github.com/grafana/pyroscope-go/godeltaprof v0.1.11 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 // indirect @@ -93,6 +97,7 @@ require ( github.com/xdg-go/scram v1.2.0 // indirect github.com/xdg-go/stringprep v1.0.4 // indirect github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 // indirect + github.com/yuin/gopher-lua v1.1.1 // indirect github.com/zerodha/logf v0.5.5 // indirect go.jtlabs.io/query v1.6.1 // indirect go.mongodb.org/mongo-driver/v2 v2.8.1 // indirect diff --git a/go.sum b/go.sum index 7421bc8..c731d88 100644 --- a/go.sum +++ b/go.sum @@ -12,6 +12,8 @@ github.com/KyleBanks/depth v1.2.1 h1:5h8fQADFrWtarTdtDudMmGsC7GPbOAu6RVB3ffsVFHc github.com/KyleBanks/depth v1.2.1/go.mod h1:jzSb9d0L43HxTQfT+oSA1EEp2q+ne2uh6XgeJcm8brE= github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= +github.com/alicebob/miniredis/v2 v2.39.0 h1:M7WbmV5BmV56L8KTG0rw6vEQ+woTOghpDgin2xv4A0g= +github.com/alicebob/miniredis/v2 v2.39.0/go.mod h1:TcL7YfarKPGDAthEtl5NBeHZfeUQj6OXMm/+iu5cLMM= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bits-and-blooms/bitset v1.2.0 h1:Kn4yilvwNtMACtf1eYDlG8H77R07mZSPbMjLyS07ChA= @@ -87,8 +89,15 @@ github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/golang/snappy v1.0.0 h1:Oy607GVXHs7RtbggtPBnr2RmDArIsAefDwvrdWvRhGs= github.com/golang/snappy v1.0.0/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= +github.com/gomodule/redigo v1.9.3 h1:dNPSXeXv6HCq2jdyWfjgmhBdqnR6PRO3m/G05nvpPC8= +github.com/gomodule/redigo v1.9.3/go.mod h1:KsU3hiK/Ay8U42qpaJk+kuNa3C+spxapWpM+ywhcgtw= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/go-github/v76 v76.0.0 h1:MCa9VQn+VG5GG7Y7BAkBvSRUN3o+QpaEOuZwFPJmdFA= +github.com/google/go-github/v76 v76.0.0/go.mod h1:38+d/8pYDO4fBLYfBhXF5EKO0wA3UkXBjfmQapFsNCQ= +github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= +github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= @@ -208,8 +217,8 @@ github.com/swaggo/gin-swagger v1.6.1 h1:Ri06G4gc9N4t4k8hekMigJ9zKTFSlqj/9paAQCQs github.com/swaggo/gin-swagger v1.6.1/go.mod h1:LQ+hJStHakCWRiK/YNYtJOu4mR2FP+pxLnILT/qNiTw= github.com/swaggo/swag v1.16.6 h1:qBNcx53ZaX+M5dxVyTrgQ0PJ/ACK+NzhwcbieTt+9yI= github.com/swaggo/swag v1.16.6/go.mod h1:ngP2etMK5a0P3QBizic5MEwpRmluJZPHjXcMoj4Xesg= -github.com/sweetrpg/api-core.go v0.1.1 h1:qqr5Rgiaw7n2RtDZ7fdoy5vw8RT3L42orhJ/x+EYX9w= -github.com/sweetrpg/api-core.go v0.1.1/go.mod h1:ka2gLYUlukRaBXiaaohfRSHoNH77oSrTkkik07Z+lj0= +github.com/sweetrpg/api-core.go v0.3.1 h1:Fa0dzFjoohTGLt182pO55kksScGNKi6nFHRij/68GTw= +github.com/sweetrpg/api-core.go v0.3.1/go.mod h1:Li4x24U0q67096dbvOm1Fuq/xOMG1elcgiuJAV8WTJA= github.com/sweetrpg/common.go v0.0.16 h1:XT8PwUXz0BOMIITXLjw3G6rEjjRUEs0Xs4+R9o2fVBk= github.com/sweetrpg/common.go v0.0.16/go.mod h1:PVpjDIeB/bZYJZk5fc8whuqVJE8K5k/DsO3o+aZFEcE= github.com/sweetrpg/mongodb.go v0.0.193 h1:9SvQK3QYftFP1GglMrMvBBlWbIAnSnjQ2Vg13uxQLkQ= @@ -229,6 +238,8 @@ github.com/xdg-go/stringprep v1.0.4/go.mod h1:mPGuuIYwz7CmR2bT9j4GbQqutWS1zV24gi github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 h1:ilQV1hzziu+LLM3zUTJ0trRztfwgjqKnBWNtSRkbmwM= github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78/go.mod h1:aL8wCCfTfSfmXjznFBSZNN13rSJjlIOI1fUNAtF7rmI= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= +github.com/yuin/gopher-lua v1.1.1 h1:kYKnWBjvbNP4XLT3+bPEwAXJx262OhaHDWDVOPjL46M= +github.com/yuin/gopher-lua v1.1.1/go.mod h1:GBR0iDaNXjAgGg9zfCvksxSRnQx76gclCIb7kdAd1Pw= github.com/zerodha/logf v0.5.5 h1:AhxHlixHNYwhFjvlgTv6uO4VBKYKxx2I6SbHoHtWLBk= github.com/zerodha/logf v0.5.5/go.mod h1:HWpfKsie+WFFpnUnUxelT6Z0FC6xu9+qt+oXNMPg6y8= go.jtlabs.io/query v1.6.1 h1:AldVImTH2RR5M9h8fCdZNyN7bew5JMVuDPelUQnP0DE= @@ -323,6 +334,7 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc golang.org/x/tools v0.48.0 h1:3+hClM1aLL5mjMKm5ovokw9epgRXPuu2tILgismM6RE= golang.org/x/tools v0.48.0/go.mod h1:08xX0orndb/F7jJxGDicx061tyd5pcMto75YMAXr6lk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa h1:Kjn0N0tCrDgiAFW+lGO4JZ3ck44CehvJQMAwj9QF0G8= diff --git a/server/feedback.go b/server/feedback.go new file mode 100644 index 0000000..43281d6 --- /dev/null +++ b/server/feedback.go @@ -0,0 +1,130 @@ +package server + +import ( + "fmt" + "net/http" + "strings" + "time" + + "github.com/gin-gonic/gin" + "github.com/sweetrpg/admin-api/constants" + "github.com/sweetrpg/admin-api/feedback" + apiv "github.com/sweetrpg/api-core.go/vo" + "github.com/sweetrpg/common.go/logging" +) + +// feedbackTypeBug and feedbackTypeFeature are the only accepted values for createFeedbackRequest.Type. +const ( + feedbackTypeBug = "bug" + feedbackTypeFeature = "feature" +) + +// createFeedbackRequest is the payload accepted by POST /feedback. Website and RenderedAt back +// the bot-detection check (honeypot + minimum fill-time); real users never populate Website +// and never submit faster than FeedbackMinFillTime after RenderedAt - see design.md. +type createFeedbackRequest struct { + Type string `json:"type" binding:"required" example:"bug"` + Title string `json:"title" binding:"required" example:"Catalog search returns no results for exact ISBN match"` + Body string `json:"body" binding:"required" example:"Searching for 9780786965601 returns zero results, even though the volume exists in the catalog."` + ReporterEmail string `json:"reporter_email,omitempty" example:"reporter@example.com"` + Source string `json:"source,omitempty" example:"catalog-web:/catalog/search"` + Website string `json:"website,omitempty"` + RenderedAt *time.Time `json:"rendered_at,omitempty"` +} + +// feedbackAcceptedResponse is the uniform success body - identical whether a GitHub Issue was +// actually created or the submission was silently discarded as bot traffic. Deliberately gives +// a bot no signal about which outcome occurred. +type feedbackAcceptedResponse struct { + Status string `json:"status" example:"accepted"` +} + +func setupFeedbackHandlers(g *gin.Engine, issueCreator feedback.IssueCreator, rateLimit gin.HandlerFunc) { + logging.Logger.Info("Setting up feedback endpoint handlers...") + + g.POST("/feedback", rateLimit, createFeedback(issueCreator)) +} + +// Submit feedback. +// +// @Summary Submit feedback +// @Description Accept an anonymous bug report or feature request and file it as a GitHub Issue on sweetrpg/platform, labeled source:in-app-feedback plus bug or feature. No GitHub credentials are ever requested from or exposed to the caller. Subject to a feedback-specific rate limit stricter than this service's default. +// @Tags feedback +// @Accept json +// @Produce json +// @Param body body createFeedbackRequest true "Feedback submission" +// @Success 200 {object} feedbackAcceptedResponse +// @Failure 400 {object} apiv.ErrorVO "missing/invalid type, title, or body, or title/body exceeds the length cap" +// @Failure 429 {object} apiv.ErrorVO "feedback-specific rate limit exceeded" +// @Failure 502 {object} apiv.ErrorVO "GitHub API call failed or timed out" +// @Failure 503 {object} apiv.ErrorVO "rate-limit backend unavailable" +// @Router /feedback [post] +func createFeedback(issueCreator feedback.IssueCreator) gin.HandlerFunc { + return func(c *gin.Context) { + var req createFeedbackRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, apiv.ErrorVO{Error: "invalid_request", Message: err.Error()}) + return + } + + if req.Type != feedbackTypeBug && req.Type != feedbackTypeFeature { + c.JSON(http.StatusBadRequest, apiv.ErrorVO{Error: "invalid_type", Message: "type must be \"bug\" or \"feature\""}) + return + } + if strings.TrimSpace(req.Title) == "" || strings.TrimSpace(req.Body) == "" { + c.JSON(http.StatusBadRequest, apiv.ErrorVO{Error: "invalid_request", Message: "title and body are required"}) + return + } + if len(req.Title) > constants.FeedbackTitleMaxLength { + c.JSON(http.StatusBadRequest, apiv.ErrorVO{Error: "title_too_long", Message: fmt.Sprintf("title exceeds %d characters", constants.FeedbackTitleMaxLength)}) + return + } + if len(req.Body) > constants.FeedbackBodyMaxLength { + c.JSON(http.StatusBadRequest, apiv.ErrorVO{Error: "body_too_long", Message: fmt.Sprintf("body exceeds %d characters", constants.FeedbackBodyMaxLength)}) + return + } + + if looksAutomated(req) { + logging.Logger.Warn("Discarding feedback submission that failed bot-detection check") + c.JSON(http.StatusOK, feedbackAcceptedResponse{Status: "accepted"}) + return + } + + body := issueBody(req) + labels := []string{constants.FeedbackSourceLabel, req.Type} + if err := issueCreator.CreateIssue(c.Request.Context(), req.Title, body, labels); err != nil { + logging.Logger.Error("Failed to create GitHub issue from feedback submission", "error", err.Error()) + c.JSON(http.StatusBadGateway, apiv.ErrorVO{Error: "github_unavailable", Message: "failed to record feedback; please try again shortly"}) + return + } + + c.JSON(http.StatusOK, feedbackAcceptedResponse{Status: "accepted"}) + } +} + +// looksAutomated reports whether a submission fails the bot-detection check: a populated +// honeypot field (real users never see or fill it), or submission faster than +// constants.FeedbackMinFillTime after the client-reported render time. +func looksAutomated(req createFeedbackRequest) bool { + if req.Website != "" { + return true + } + if req.RenderedAt != nil && time.Since(*req.RenderedAt) < constants.FeedbackMinFillTime { + return true + } + return false +} + +// issueBody renders the submitted body plus any optional fields, appended only when present +// per the "Submission without optional fields" scenario - no empty placeholders. +func issueBody(req createFeedbackRequest) string { + var b strings.Builder + b.WriteString(req.Body) + if req.ReporterEmail != "" { + fmt.Fprintf(&b, "\n\n---\nReporter email: %s", req.ReporterEmail) + } + if req.Source != "" { + fmt.Fprintf(&b, "\n\nSource: %s", req.Source) + } + return b.String() +} diff --git a/server/feedback_test.go b/server/feedback_test.go new file mode 100644 index 0000000..fa81518 --- /dev/null +++ b/server/feedback_test.go @@ -0,0 +1,287 @@ +package server + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/alicebob/miniredis/v2" + "github.com/gin-gonic/gin" + "github.com/gomodule/redigo/redis" + "github.com/sweetrpg/admin-api/feedback" + "github.com/sweetrpg/common.go/logging" +) + +func TestMain(m *testing.M) { + logging.Init() + m.Run() +} + +// fakeIssueCreator records calls and returns a configurable error, so tests can assert both +// "an issue was created with these labels/title/body" and "GitHub failure surfaces as 502" +// without making a real network call. +type fakeIssueCreator struct { + called bool + title string + body string + labels []string + err error +} + +func (f *fakeIssueCreator) CreateIssue(_ context.Context, title, body string, labels []string) error { + f.called = true + f.title = title + f.body = body + f.labels = labels + return f.err +} + +func newFeedbackTestRouter(creator feedback.IssueCreator) *gin.Engine { + gin.SetMode(gin.TestMode) + r := gin.New() + setupFeedbackHandlers(r, creator, func(c *gin.Context) { c.Next() }) + return r +} + +func postFeedback(r *gin.Engine, body map[string]any) *httptest.ResponseRecorder { + b, _ := json.Marshal(body) + req := httptest.NewRequest(http.MethodPost, "/feedback", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + return w +} + +func TestCreateFeedback_ValidBugReport(t *testing.T) { + creator := &fakeIssueCreator{} + r := newFeedbackTestRouter(creator) + + w := postFeedback(r, map[string]any{ + "type": "bug", + "title": "Catalog search broken", + "body": "Searching for an exact ISBN returns no results.", + }) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) + } + if !creator.called { + t.Fatal("expected CreateIssue to be called for a valid submission") + } + if creator.title != "Catalog search broken" { + t.Errorf("issue title = %q, want %q", creator.title, "Catalog search broken") + } + wantLabels := []string{"source:in-app-feedback", "bug"} + if len(creator.labels) != 2 || creator.labels[0] != wantLabels[0] || creator.labels[1] != wantLabels[1] { + t.Errorf("issue labels = %v, want %v", creator.labels, wantLabels) + } +} + +func TestCreateFeedback_MissingOrInvalidFields(t *testing.T) { + cases := []struct { + name string + body map[string]any + }{ + {"missing type", map[string]any{"title": "t", "body": "b"}}, + {"invalid type", map[string]any{"type": "complaint", "title": "t", "body": "b"}}, + {"missing title", map[string]any{"type": "bug", "body": "b"}}, + {"missing body", map[string]any{"type": "bug", "title": "t"}}, + {"blank title", map[string]any{"type": "bug", "title": " ", "body": "b"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + creator := &fakeIssueCreator{} + r := newFeedbackTestRouter(creator) + + w := postFeedback(r, tc.body) + + if w.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body = %s", w.Code, w.Body.String()) + } + if creator.called { + t.Error("expected CreateIssue not to be called for a rejected submission") + } + }) + } +} + +func TestCreateFeedback_OversizedFieldsRejected(t *testing.T) { + longTitle := make([]byte, 201) + for i := range longTitle { + longTitle[i] = 'a' + } + longBody := make([]byte, 10001) + for i := range longBody { + longBody[i] = 'b' + } + + creator := &fakeIssueCreator{} + r := newFeedbackTestRouter(creator) + + w := postFeedback(r, map[string]any{ + "type": "feature", + "title": string(longTitle), + "body": "ok", + }) + if w.Code != http.StatusBadRequest { + t.Fatalf("oversized title: status = %d, want 400", w.Code) + } + + w = postFeedback(r, map[string]any{ + "type": "feature", + "title": "ok", + "body": string(longBody), + }) + if w.Code != http.StatusBadRequest { + t.Fatalf("oversized body: status = %d, want 400", w.Code) + } + if creator.called { + t.Error("expected CreateIssue not to be called for an oversized submission") + } +} + +func TestCreateFeedback_HoneypotPopulated_SilentlyAccepted(t *testing.T) { + creator := &fakeIssueCreator{} + r := newFeedbackTestRouter(creator) + + w := postFeedback(r, map[string]any{ + "type": "bug", + "title": "t", + "body": "b", + "website": "http://spam.example", + }) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (silent accept); body = %s", w.Code, w.Body.String()) + } + if creator.called { + t.Error("expected CreateIssue not to be called when honeypot is populated") + } +} + +func TestCreateFeedback_SubmittedTooFast_SilentlyAccepted(t *testing.T) { + creator := &fakeIssueCreator{} + r := newFeedbackTestRouter(creator) + + renderedAt := time.Now() + w := postFeedback(r, map[string]any{ + "type": "bug", + "title": "t", + "body": "b", + "rendered_at": renderedAt.Format(time.RFC3339Nano), + }) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (silent accept); body = %s", w.Code, w.Body.String()) + } + if creator.called { + t.Error("expected CreateIssue not to be called for a too-fast submission") + } +} + +func TestCreateFeedback_OptionalFieldsIncludedInBody(t *testing.T) { + creator := &fakeIssueCreator{} + r := newFeedbackTestRouter(creator) + + w := postFeedback(r, map[string]any{ + "type": "feature", + "title": "t", + "body": "b", + "reporter_email": "reporter@example.com", + "source": "main-web:/", + }) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) + } + if !creator.called { + t.Fatal("expected CreateIssue to be called") + } + if !contains(creator.body, "reporter@example.com") || !contains(creator.body, "main-web:/") { + t.Errorf("issue body = %q, want it to include both optional fields", creator.body) + } +} + +func TestCreateFeedback_OptionalFieldsOmittedWhenAbsent(t *testing.T) { + creator := &fakeIssueCreator{} + r := newFeedbackTestRouter(creator) + + w := postFeedback(r, map[string]any{"type": "bug", "title": "t", "body": "b"}) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) + } + if creator.body != "b" { + t.Errorf("issue body = %q, want exactly the submitted body with no placeholders", creator.body) + } +} + +func TestCreateFeedback_GitHubFailure_Returns502(t *testing.T) { + creator := &fakeIssueCreator{err: errors.New("github: 500 internal server error")} + r := newFeedbackTestRouter(creator) + + w := postFeedback(r, map[string]any{"type": "bug", "title": "t", "body": "b"}) + + if w.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want 502; body = %s", w.Code, w.Body.String()) + } +} + +func contains(s, substr string) bool { + return len(s) >= len(substr) && (func() bool { + for i := 0; i+len(substr) <= len(s); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false + })() +} + +// TestFeedbackRateLimit_ExceedsBudgetWithoutAffectingOtherRoutes exercises +// feedback.RateLimitMiddleware directly against a miniredis-backed pool: once the feedback +// tier's budget is exhausted, /feedback 429s while an unrelated route mounted alongside it +// keeps responding normally. +func TestFeedbackRateLimit_ExceedsBudgetWithoutAffectingOtherRoutes(t *testing.T) { + t.Setenv("RATE_LIMIT_FEEDBACK", "1") + t.Setenv("RATE_LIMIT_FEEDBACK_WINDOW_SECONDS", "60") + + mr := miniredis.RunT(t) + pool := &redis.Pool{ + Dial: func() (redis.Conn, error) { + return redis.Dial("tcp", mr.Addr()) + }, + } + t.Cleanup(func() { _ = pool.Close() }) + + gin.SetMode(gin.TestMode) + r := gin.New() + creator := &fakeIssueCreator{} + setupFeedbackHandlers(r, creator, feedback.RateLimitMiddleware(pool)) + r.GET("/other", func(c *gin.Context) { c.Status(http.StatusOK) }) + + body := map[string]any{"type": "bug", "title": "t", "body": "b"} + + w1 := postFeedback(r, body) + if w1.Code != http.StatusOK { + t.Fatalf("first submission: status = %d, want 200; body = %s", w1.Code, w1.Body.String()) + } + + w2 := postFeedback(r, body) + if w2.Code != http.StatusTooManyRequests { + t.Fatalf("second submission: status = %d, want 429; body = %s", w2.Code, w2.Body.String()) + } + + other := httptest.NewRequest(http.MethodGet, "/other", nil) + otherW := httptest.NewRecorder() + r.ServeHTTP(otherW, other) + if otherW.Code != http.StatusOK { + t.Errorf("unrelated route status = %d, want 200 - feedback rate limit must not affect other routes", otherW.Code) + } +} diff --git a/server/handlers.go b/server/handlers.go index 607ab6f..c83e8df 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -3,11 +3,13 @@ package server import ( "github.com/gin-gonic/gin" "github.com/sweetrpg/admin-api/authz" + "github.com/sweetrpg/admin-api/feedback" ) -func SetupHandlers(g *gin.Engine, authzClient *authz.Client) { +func SetupHandlers(g *gin.Engine, authzClient *authz.Client, issueCreator feedback.IssueCreator, feedbackRateLimit gin.HandlerFunc) { setupBannerHandlers(g, authzClient) setupMaintenanceModeHandlers(g, authzClient) setupAppCardStatusHandlers(g, authzClient) setupStatusHandlers(g) + setupFeedbackHandlers(g, issueCreator, feedbackRateLimit) } diff --git a/server/write_auth_integration_test.go b/server/write_auth_integration_test.go index 0e1b24d..741e766 100644 --- a/server/write_auth_integration_test.go +++ b/server/write_auth_integration_test.go @@ -1,6 +1,7 @@ package server import ( + "context" "net/http" "net/http/httptest" "testing" @@ -10,6 +11,12 @@ import ( "github.com/sweetrpg/common.go/logging" ) +// noopIssueCreator is a stub feedback.IssueCreator for tests that don't exercise /feedback +// itself but need a valid SetupHandlers call. +type noopIssueCreator struct{} + +func (noopIssueCreator) CreateIssue(_ context.Context, _, _ string, _ []string) error { return nil } + // TestWriteRoutesRequireAuth registers every route admin-api exposes (the // same SetupHandlers wiring main.go uses) and asserts every write route // (POST/PUT/DELETE) 401s without credentials, while every read route (GET) is @@ -20,7 +27,7 @@ func TestWriteRoutesRequireAuth(t *testing.T) { logging.Init() gin.SetMode(gin.TestMode) r := gin.New() - SetupHandlers(r, authz.NewClient("")) + SetupHandlers(r, authz.NewClient(""), noopIssueCreator{}, func(c *gin.Context) { c.Next() }) writeRoutes := []struct { method string From 870f9a20303cb83b295ce9a8c7ea6c02d32be9c3 Mon Sep 17 00:00:00 2001 From: Paul Schifferer Date: Tue, 6 Oct 2026 21:51:47 -0700 Subject: [PATCH 2/6] chore(k8s): wire api-feedback secret into admin-api deployment Adds a secretRef to the api-feedback Secret (produced by an ExternalSecret pulling /sweetrpg/admin/api/feedback from Akeyless, containing GITHUB_FEEDBACK_TOKEN) alongside the existing api-db/ api-misc secretRefs, matching this repo's existing pattern for secret-backed env vars. The ExternalSecret manifest itself (kubernetes/overlays/dev/secrets/ feedback.yaml) still needs to be added by someone with access to that path - this session's sandbox denies all read/write access to any **/secrets path, so kustomization.yaml does not yet list it. --- kubernetes/base/deployment.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kubernetes/base/deployment.yaml b/kubernetes/base/deployment.yaml index 93d66d3..aa9f55b 100644 --- a/kubernetes/base/deployment.yaml +++ b/kubernetes/base/deployment.yaml @@ -43,6 +43,8 @@ spec: name: api-db - secretRef: name: api-misc + - secretRef: + name: api-feedback env: - name: GOMEMLIMIT value: "900MiB" From da77113a75df4c61be26178212ba65229f954bc0 Mon Sep 17 00:00:00 2001 From: Paul Schifferer Date: Wed, 7 Oct 2026 06:36:39 -0700 Subject: [PATCH 3/6] feat(kubernetes): wire feedback rate-limiter to Redis cache Add Redis cache configuration (REDIS_HOST/REDIS_PORT) to admin-api's development deployment, enabling the feedback endpoint's feedback-tier rate limiter to back against a real cache instance instead of failing closed with 503. Wire the cache auth secret (REDIS_PASS) via ExternalSecret pulling from Akeyless. Resolves sweetrpg/platform#112. Co-Authored-By: Claude Haiku 4.5 --- kubernetes/base/deployment.yaml | 2 ++ kubernetes/overlays/dev/configmaps.yaml | 4 ++++ kubernetes/overlays/dev/kustomization.yaml | 1 + kubernetes/overlays/dev/secrets/cache.yaml | 18 ++++++++++++++++++ 4 files changed, 25 insertions(+) create mode 100644 kubernetes/overlays/dev/secrets/cache.yaml diff --git a/kubernetes/base/deployment.yaml b/kubernetes/base/deployment.yaml index aa9f55b..a5ca359 100644 --- a/kubernetes/base/deployment.yaml +++ b/kubernetes/base/deployment.yaml @@ -45,6 +45,8 @@ spec: name: api-misc - secretRef: name: api-feedback + - secretRef: + name: api-cache env: - name: GOMEMLIMIT value: "900MiB" diff --git a/kubernetes/overlays/dev/configmaps.yaml b/kubernetes/overlays/dev/configmaps.yaml index 1056783..c7d506c 100644 --- a/kubernetes/overlays/dev/configmaps.yaml +++ b/kubernetes/overlays/dev/configmaps.yaml @@ -22,3 +22,7 @@ data: # PYROSCOPE_TENANT_ID: psw SENTRY_RELEASE: "1" AUTH_API_URL: http://api-v1.sweetrpg-auth.svc.cluster.local:8000 + # Namespace cache Service (support/infrastructure admin/cache, clusters/dev/admin/cache.yaml). + # Backs the feedback endpoint's rate-limit counters. + REDIS_HOST: cache.sweetrpg-admin.svc.cluster.local + REDIS_PORT: "6379" diff --git a/kubernetes/overlays/dev/kustomization.yaml b/kubernetes/overlays/dev/kustomization.yaml index 8961690..91b1d18 100644 --- a/kubernetes/overlays/dev/kustomization.yaml +++ b/kubernetes/overlays/dev/kustomization.yaml @@ -14,6 +14,7 @@ resources: - secrets/db-creds.yaml - secrets/db.yaml - secrets/misc.yaml + - secrets/cache.yaml labels: - pairs: diff --git a/kubernetes/overlays/dev/secrets/cache.yaml b/kubernetes/overlays/dev/secrets/cache.yaml new file mode 100644 index 0000000..43e9af0 --- /dev/null +++ b/kubernetes/overlays/dev/secrets/cache.yaml @@ -0,0 +1,18 @@ +--- +apiVersion: external-secrets.io/v1 +kind: ExternalSecret + +metadata: + name: admin-api-cache + +spec: + refreshInterval: 1h + secretStoreRef: + name: akeyless + kind: ClusterSecretStore + target: + name: api-cache + creationPolicy: Owner + dataFrom: + - extract: + key: /sweetrpg/admin/cache/auth From 3c2ce9ecee1559ad7674bfbb759dd478d5f7a655 Mon Sep 17 00:00:00 2001 From: Paul Schifferer Date: Wed, 7 Oct 2026 06:43:14 -0700 Subject: [PATCH 4/6] chore: do the robot's work for it --- kubernetes/overlays/dev/kustomization.yaml | 3 ++- kubernetes/overlays/dev/secrets/feedback.yaml | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 kubernetes/overlays/dev/secrets/feedback.yaml diff --git a/kubernetes/overlays/dev/kustomization.yaml b/kubernetes/overlays/dev/kustomization.yaml index 91b1d18..25a962e 100644 --- a/kubernetes/overlays/dev/kustomization.yaml +++ b/kubernetes/overlays/dev/kustomization.yaml @@ -11,10 +11,11 @@ resources: - middlewares.yaml - password.yaml - pod-monitor.yaml + - secrets/cache.yaml - secrets/db-creds.yaml - secrets/db.yaml + - secrets/feedback.yaml - secrets/misc.yaml - - secrets/cache.yaml labels: - pairs: diff --git a/kubernetes/overlays/dev/secrets/feedback.yaml b/kubernetes/overlays/dev/secrets/feedback.yaml new file mode 100644 index 0000000..cd4bec8 --- /dev/null +++ b/kubernetes/overlays/dev/secrets/feedback.yaml @@ -0,0 +1,18 @@ +--- +apiVersion: external-secrets.io/v1 +kind: ExternalSecret + +metadata: + name: admin-api-feedback + +spec: + refreshInterval: 1h + secretStoreRef: + name: akeyless + kind: ClusterSecretStore + target: + name: api-feedback + creationPolicy: Owner + dataFrom: + - extract: + key: /sweetrpg/admin/api/feedback From bff91dc7ca7d0e928a5c01d7b767769c8a913aa7 Mon Sep 17 00:00:00 2001 From: Paul Schifferer Date: Wed, 7 Oct 2026 07:21:50 -0700 Subject: [PATCH 5/6] chore(kubernetes): allow dev.sweetrpg.com origin for feedback endpoint CORS --- kubernetes/overlays/dev/configmaps.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/kubernetes/overlays/dev/configmaps.yaml b/kubernetes/overlays/dev/configmaps.yaml index c7d506c..b570645 100644 --- a/kubernetes/overlays/dev/configmaps.yaml +++ b/kubernetes/overlays/dev/configmaps.yaml @@ -26,3 +26,7 @@ data: # Backs the feedback endpoint's rate-limit counters. REDIS_HOST: cache.sweetrpg-admin.svc.cluster.local REDIS_PORT: "6379" + # main-web's feedback widget is the first browser-side (cross-origin) caller of this API - + # every prior consumer (main-web, catalog-web) fetches server-side, which setupCORS never + # gated. Without this, the Gin CORS middleware stays disabled entirely. + ALLOWED_ORIGINS: https://dev.sweetrpg.com From 8ed81cafa2c8e2d8e94558d0451135673e200728 Mon Sep 17 00:00:00 2001 From: Paul Schifferer Date: Wed, 7 Oct 2026 07:53:43 -0700 Subject: [PATCH 6/6] style: fix import ordering in feedback/ratelimit.go (gofmt) --- feedback/ratelimit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/feedback/ratelimit.go b/feedback/ratelimit.go index c4c1864..c8849df 100644 --- a/feedback/ratelimit.go +++ b/feedback/ratelimit.go @@ -5,11 +5,11 @@ import ( "github.com/gin-gonic/gin" "github.com/gomodule/redigo/redis" + "github.com/sweetrpg/admin-api/constants" apicoreconstants "github.com/sweetrpg/api-core.go/constants" "github.com/sweetrpg/api-core.go/ratelimit" "github.com/sweetrpg/api-core.go/util" apiv "github.com/sweetrpg/api-core.go/vo" - "github.com/sweetrpg/admin-api/constants" "github.com/sweetrpg/common.go/logging" )