Repository navigation
[security] Authenticate avatar generation and cap anonymous Replicate cost (closes #44) - #46
Conversation
…cate cost (closes Harrolee#44) coach-avatar-generator was callable by anyone who knew the URL. Every request ran generateCoachAvatars, which fans out to all 11 Replicate styles when style is omitted — burning the full set of credits per call with no rate limit and no ownership check. Two callers with genuinely different needs: CoachAvatarEdit (logged-in user editing an existing coach): - Sends Authorization header with Supabase JWT. - Server verifies the token and checks coach_profiles.user_id matches the caller before generating. AvatarUpload (pre-signup coach builder): - No session, by design — the builder flow is the signup funnel. - coachId must start with "temp-" so anonymous callers cannot target real coach profiles. - style is required — prevents fan-out to every style, capping anonymous Replicate cost to one generation per call. - IP rate limit (6/hour, configurable via UNAUTH_RATE_LIMIT). Additional hardening: saveSelectedAvatar now requires authentication and coach ownership. Previously any caller could overwrite any coach's avatar. CORS tightened from wildcard to ALLOWED_ORIGINS (already passed by Terraform, other functions already use it). Terraform changes: Added UNAUTH_RATE_LIMIT and UNAUTH_WINDOW_MS env vars to the coach-avatar-generator module. These are documented, reviewed HCL only — the deployer applies them.
Resolves the conflict with Harrolee#45 and fixes three things in the contributed change. Rate-limit key. The limiter read `req.ip` first and fell back to `X-Forwarded-For`. Express only populates `req.ip` from that header when `trust proxy` is set, which the functions framework does not do here, so `req.ip` is Cloud Run's front end — the same value for every caller. The per-IP ceiling would have been a global one, and the pre-signup avatar step would have started 429ing everybody after a handful of builds, which is the exact funnel this endpoint exists to serve. Now keyed on the left-most X-Forwarded-For entry. saveSelectedAvatar stays deleted. Harrolee#46 restored it with resolveCaller() and isCoachOwner() in front, which does fix the IDOR, but Terraform gives this directory one entry_point and it is not that one — so it is unreachable code on a write path, one Terraform line away from being live. Both helpers remain, so bringing saving back is a small change; do it with an entry_point and a caller in the same PR. AVATAR_STYLES is required at the top rather than lazily inside the 400 branch. Adds mobile/e2e/avatar-auth-probe.mjs — 25 checks over the authorization matrix, which Harrolee#44 asked for and the PR did not have. It loads the real index.js with ./avatar-generation replaced by a recording stub, the technique crisis-probe.mjs uses on openai, so it spends no Replicate credits and can assert the part that matters: a rejected call reaches generation zero times. A 403 that still burned a credit looks identical from the outside. Supabase is deliberately not stubbed — token verification and the ownership lookup run against the real stack. Verified the probe can fail: disabling isCoachOwner() and the temp- prefix check turns 25/0 into 18/7, including an anonymous caller generating against a real coach id. Co-Authored-By: cekuu35 <cekuu35@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`function-gateway.js` read `replicate_api_key` from `_infra/terraform.tfvars` unconditionally, with no equivalent of the `USE_REAL_OPENAI=1` gate that guards the chat API. Every local `viz-realtime-probe.mjs` run therefore made real, billed image predictions against the production Replicate account. The probe's own comments said the opposite — "without spending anything at Replicate ... the runs below fail at the Replicate boundary (no token in the harness)" — so nobody reading the suite had reason to think it cost money. It also made that suite slow and unreliable, because its assertions ended up depending on a third-party network round trip. Found while investigating intermittent failures: with the token in place a run took over ten minutes and the gateway log showed real predictions failing on `example.com` placeholder URLs. With it gated the same run takes 2m43s. Replicate is now opt-in via `USE_REAL_REPLICATE=1`, symmetric with OpenAI, and the gateway refuses to start if you ask for it without a key. Off by default the token is unset and predictions fail immediately at the boundary — which is what the likeness assertions already assumed, since `coach_visualizations.model` is written before the call and the model choice is what is under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks for this — the shape is right, and splitting the authenticated path from the pre-signup one is the correct call. I've merged Rate-limit key. The limiter read
Added I checked the probe can actually fail: disabling Two of your choices I verified rather than assumed: Unrelated but found while verifying this: Verified: 403 checks green across eight probe suites, plus |
Summary
coach-avatar-generatoris publicly callable — anyone who knows the URL can POST a selfie and burn Replicate credits. Every request runsgenerateCoachAvatars, which fans out to all 11 styles whenstyleis omitted.This PR splits the endpoint into two auth paths, matching the two callers documented in the issue:
Authenticated path (
CoachAvatarEdit— logged-in user)Authorization: Bearer <supabase JWT>headersupabase.auth.getUser(token)to verify the callercoach_profiles.user_idmatches the authenticated user before generatingAnonymous path (
AvatarUpload— pre-signup builder)coachIdmust start withtemp-— anonymous callers cannot target real coachesstyleis required — prevents fan-out to every style, capping Replicate cost to one generation per call (the key blast-radius cap from the issue)UNAUTH_RATE_LIMIT)Retry-After) on 429Additional hardening
saveSelectedAvatarnow requires authentication and coach ownership (previously callable by anyone)*toALLOWED_ORIGINS(already passed by Terraform, every other function in this repo already uses it)Terraform
Added
UNAUTH_RATE_LIMITandUNAUTH_WINDOW_MSenv vars to thecoach-avatar-generatormodule. Reviewed HCL only — the deployer applies them.Closes #44